DaedTech

Stories about Software

Jun
23

By

Code Review and How Enterprises Can Miss The Point

Category: Language Agnostic Tags: | 4 Comments

Editorial Note: I originally wrote this post for the SmartBear blog.  You can check out the original here, at their site.  Check out my posts and some of the others and take a look at their products as well while you’re there.

If you work for a smallish company, as part of a modestly sized software development group, the path to a code review policy is likely a short, direct one.  It could be as simple as a respected team member or the manager saying, “hey, let’s start doing code review.”  But whatever the impetus, the participants will tie the process closely enough to the desired outcomes from it to adapt it as needed.

The Capital E Enterprise

At the enterprise level, the calculus changes considerably. And when I say the enterprise, I mean The Enterprise – a size and scope mammoth enough to demand capitalization, even when the rules of grammar do not. These are companies so big that those who work at and have worked at them will assure you that there is simply nothing out there that’s comparable. There are, they will tell you, an entirely different set of rules that apply. And they’re more or less right.

The Enterprise loves structure and hierarchy, usually of the command and control variety. The scale is so immense that the only structure up to the task is a pyramid reminiscent of a military chain of command. The organization lumbers along like a battleship, powerful, majestic, and requiring incredible teamwork, cooperation, and precision to change direction in any way.

GoofyOrgChart

In this environment, code review tends to make its way to development team in a very different way and for very different reasons. Of course, even in a massively homogenized environment, one size does not fit all for the development teams. But the cog in a larger machine dynamic makes the following sort of scenario much more likely.

Read More

Jun
9

By

Creating Your Code Review Checklist

Category: The Life of a Programmer Tags: , | 5 Comments

Editorial Note: I originally wrote this post for the SmartBear blog.  You can read the original here, at their site.  There’s a lot of other good stuff over there as well, so look around.

In any line of work, there are a number of rites of passage by which you can mark your career and feel good about milestones achieved.  Programming is no different.

Think back to the first time you were asked to perform a code review.  How exciting! (Though you probably laugh at your younger self now for being excited to review code.)  You spent time as the newest programmer in the organization, under the impression that code reviews were something done to you rather than by you, but now you have confirmation that your opinion is valued.

If you’re anything like me, the first thing you did in such a situation was to scramble to Google and start punching in things like, “how to conduct a code review,” “code review checklist,” or “code review best practices.” Being asked to weigh in on someone’s code is a lot of responsibility, and who wants to screw that up?  In such a Google search, you’ll probably find recommendations that steer you toward a list like this:

  • Does every method have an XML comment?
  • Do classes have a copyright header?
  • Do fields, methods, and types follow our standard naming convention?
  • Do methods have too many parameters?
  • Are you checking validity of method parameters?
  • Does the code have “magic” values instead of named constants?

Seems pretty reasonable, right?  If we brainstormed for an hour or two, we could probably flesh this out to be a comprehensive list with maybe 100 items on it.  And that’s what a lot of code review checklists look like — large lists of things for reviewers to go through and check.  So code review becomes an activity reminiscent of a line supervisor inspecting factory equipment with a clipboard.

Clipboard

There are two problems with this.

  1. You can’t keep 100+ items in your head as you look at every method or clause in a code base, so you’re going to have to read the code over and over, looking for different things.
  2. None of the checks I listed above actually require human intervention. They can all be handled via static analysis.

Read More

May
8

By

Github and Code Review: A Quiet Revolution

Category: The Life of a Programmer Tags: , |

Editorial Note: I originally wrote this post for the SmartBear blog.  Go check out the original here, at their site.  Stick around and check out some of the other authors and posts over there if you’re so inclined.

When the winds of change blow through the programming world, they don’t necessarily hit everyone with equal force.  The start-up folks cranking out reams of Ruby code on their Macs probably feel a gale-force headwind, while a Software Engineer III toiling away in Java 1.5 for some Fortune 500 bank might feel only the slightest breeze.  But on a long enough timeline, the wind changes things for everyone.

Github has proven nothing short of a revolution for a lot of small, nimble organizations, startups, and cutting edge companies.  For heavily regulated, locked down enterprises, this effect is certainly muted, but I would argue that its subtly perceptible nonetheless.  Github is changing a lot of things about software development, and this includes the nature of code review.

Let’s consider some properties of Github.

Social Coding

Github is often described as a social network for programmers.  The term “social coding” has even appeared in some of Github’s marketing material.  It is a platform meant, specifically, for maximum interaction.

Sure, Github is a vehicle for open source contributions, but that’s hardly a difference-maker for them.  Sourceforge was around for a long time and it would host source control for open source projects for free.  There have also been other communities oriented around contributions and code sharing, such as Code Project.  Github, however, came along and truly married social with coding, introducing feeds, followers, ubiquitous collaboration tools, and even a social network graph.  Having cute octopus buddies as their mascots probably didn’t hurt matters either.

HighFive

The result is an unprecedented amount of enthusiasm for global sharing of code.  20 or even 10 years ago, you probably would have hoarded source code of a side project.  You wouldn’t have wanted to give away your intellectual property and you’d probably also have been embarrassed until you could tidy it up to put your best foot forward.  Now the default is to throw your side work up on Github and show it off for the world and your followers to see.  Code is being shared as never before.

Read More

May
1

By

Is There Value in Having Non-Technical People Do Code Review?

Category: Language Agnostic Tags: | 4 Comments

Editorial note: I originally wrote this post for the SmartBear blog.  Go to their site and check out the original!  If you like this post, there are a lot of good ones there by a variety of authors, on topics like code review, API design, testing, and more.

Here’s a thought exercise for you. Should non-technical people participate in code reviews?

It’s off the beaten path, to be sure, but I think it’s an interesting philosophical consideration. We’re entirely used to code review as an exercise by developers and for developers. But is there a place or purpose for outsiders to review our code?

Why do it?

I’ll state up front my answer to that question: “yes, provided it happens in a specific, directed way.” But to convince you, let me offer some potential benefits that I see, depending on who reviews what.

  • In general, it could bring members of the team with different skill sets closer together. Developers learn the business domain; why not let the business people understand the developers’ world?
  • This could serve as a sanity check. Are developers writing code that accurately reflects the domain?
  • It could also force developers to write code that is cleaner, more readable, and more maintainable. Imagine having to write code that a non-technical person might understand.

I’ll offer more detailed rationale for this thinking shortly.  But I imagine you’d agree that these goals would be worth pursuing.  If an occasional, different style of code review can help, then it’d be worth doing.

Be careful with this.

Dragons

But before I talk about what these reviews might look like and how they could help, it’s important to stress that I’m not proposing a radical change to the code review process.  What I’m proposing is an occasional exercise to offer a different perspective on the team’s code.  Having non-technical folks look at the code shouldn’t be a vehicle for micromanagement or for former techies to quibble over code.  It shouldn’t be exhaustive, since a lot of plumbing code will be nonsense to them.

Read More

Apr
25

By

Why You Should Do Periodic Reviews of Legacy Code

Category: Language Agnostic Tags: , | 10 Comments

Editorial Note: I originally wrote this post for the SmartBear blog.  Go take a look at the original here, at their site.  If you like posts about collaboration, code review, and other topics, take a look around while you’re there.

Legacy code is sort of like your house’s storage crawlspace.  It tends to be a repository for things that mattered to you in days past or on special occasions.  The code sits there, largely unnoticed, until such time as an odd change or a production bug causes you to dig it up, dust it off, and revisit it.  Barring extraordinary circumstances, it tends to sit, largely forgotten, and possibly rotting or getting riddled with moth holes.

By and large, this considered an acceptable and even desirable state of affairs in our industry.  A lot of folks that manage software development efforts and hold the purse strings think of software construction the way they think of building construction.  Once you’ve built a house, it’s done.  Why would you go back and revisit it unless there was some kind of problem that had cropped up?  The problem with this well-intentioned, bottom-line thinking is that building software isn’t much like building houses.

When you build software, you stack the new atop the old and everything comes along for the ride.  Sure, there may be the occasional new module that stands all by itself or plugs in with the rest, but that’s the exception.  The new code interacts with the old stuff, calling into it, relying on it, and running beside it in production.  If housing construction worked this way, a short circuit in the house across the street might cause your shower to stop working.

The result is that, however well-intentioned someone encouraging you not to focus on legacy code might be, the edict is often misguided.  If the short circuit across the street meant you couldn’t shower, would you listen to someone who told you their wiring was none of your business?  Clearly, you wouldn’t go storming over there out of nowhere, remodeling that house, but you wouldn’t just ignore it either.

OldHouse

This is the approach required with legacy code in your code base.  The fact that someone typed it out years ago doesn’t mean that it doesn’t have a very real, current impact on your team every time you deploy your code.  Because of this, it behooves you to review it occasionally, when time permits.

Let’s examine some specifics as to why it makes sense to audit your legacy code from time to time.  Having your finger on the pulse of everything going into production is a compelling but abstract argument.  So, let’s get more concrete.

Read More

« Older Posts
Newer Posts »