CXO

Five pointers for conducting a successful formal code review

The formal code review process can seem overbearing and is often unpleasant, but it doesn't have to be. Follow this guide to a better, more productive review process.


You can do many things to improve the quality of your programs, but in my experience nothing produces a more dramatic result than a simple code review. These examinations of your work come in a variety of sizes and flavors, but the purpose and general approach is the same: to improve the quality of the work by having another set of eyes look at it. Although I’ll focus on the review of program code, the process is basically the same for design reviews, test plan reviews, or any other type of review you might consider conducting.

Unlike the informal review, which can be as simple as asking a colleague in the break room to take a peek at a change you’ve made, the formal code review is fairly involved. (For more on the informal approach, see Lamont Adams’ discussion in “The case for informal peer review in a development organization.”) The biggest complicating factor in a formal code review is the number of people involved. This alone produces coordination problems and increases the amount of preparation. On the other hand, having more people in the mix also produces an opportunity for you to get valuable feedback and to share information with others involved in the project.

Points to consider
To make the formal review process a success, it’s important that you manage the process to the best of your ability. There will always be some aspect of the process that will be beyond your control, but the more you can stay on top of things, the better off you’ll be. Lets look at a few of the basic considerations you should think about if you intend to manage a formal review and bring it to a successful conclusion.

Team selection
Probably the most important part of a formal code review is the selection of the review team. There’s only one rule:

Only include people who add value to the process.

I would like to call it “Jerry’s rule,” but I’m sure this has been said before. At best, having the wrong people involved will waste everyone’s time. In the worst case, it will mire the overall process, sour the participants on future reviews, and cause hard feelings that can ultimately cost someone his or her job.

Should managers be invited? A definite…depends. I have worked for both technical and nontechnical managers who not only provided valuable information to the team, but who also knew where their expertise ended. I have also worked for a manager who moved up from the technical ranks and never figured out that she was no longer a programmer. I would invite the first two managers in a heartbeat. The latter manager didn’t need to be invited because she invited herself, and the resulting session was assured of being arduous and unproductive.

Who presents?
The issue of who should present never fails to start a debate. One side holds that a stranger (i.e., not the programmer who wrote the code) will do a better, unbiased, job of presenting. Such a person can provide a naïve insight to the process.

My opinion puts me in the other camp. I believe you should present your own work. Admittedly, you may gloss over something because of your familiarity with the program (as in any proofreading situation), but that’s why the rest of the team is there. The main value I see in having developers present is that in the process of making the presentation, they’ll often spot an error that no one else has seen. The process of presenting material tends to force you to focus in a way you might not otherwise do.

Be prepared
Most of your time should be spent in preparation. Of course, you want the handouts, program listings, diagrams, flipcharts, and other meeting necessities prepared in advance so as to not waste people’s time. However, in addition to this, you should already be talking to the review team to get their impressions of bigger issues that may come up. See if they have any concerns that are real hot buttons for them. What are they primarily worried about on the project? Address all of these issues before the review meeting.

You might even consider having participants provide written comments after they review the advance material. Besides providing information for you (to fix or follow up on), an early review will hopefully force the participants to read the code before their walk to the meeting room.

What’s the point of the meeting if you do all of this up front? Plenty. Besides the exchange of knowledge and status information, completely new issues and concerns may arise from the discussion between the participants. Interface concerns, for example: Sure you wrote it to spec, and they’ve just effectively changed the specification, but it’s still better to handle these new problems now, as opposed to doing it during testing.

As a rule of thumb, if you can be so prepared that you feel the result is a forgone conclusion, you’re probably doing okay. (Just don’t think something won’t come up anyway.)

Don’t make it too long
I once had a teacher who said that your mind couldn’t keep attention longer than your butt could tolerate the chair. That’s undeniably true. So make sure the meetings don’t last longer than an hour or two. In fact, having been an instructor myself, I know that even two hours might require a break. Ideally, keep it to about an hour.

If your reviews are running longer than two hours (and the quality of the review is good), you may need to break the material into smaller parts. In other words, reduce the scope of the review. If the reviews are running longer than two hours and the quality of the session is not good, you have other problems. And until they’re solved, don’t expect to make any progress.

Review the code, not the developer
All participants in a review should feel that they can speak openly. A recent article, “Egoless programming: The path to better code,” takes a look at this issue. As a reviewer, give comments in a constructive and positive way. As the developer, don’t take comments personally. You’re all there for the same purpose.

Moving on from here
These five points are fairly basic, but they provide a good starting point for anyone looking to start a formal review process or to improve an existing one. There are, however, additional issues to consider as you move beyond the basics, such as the size of the review team and how you should actually manage or facilitate the meeting. We’ll address these aspects of the formal review process in future articles.

Reviews are a good way to improve the quality of nearly any project and are excellent when it comes to working with program code. I realize that many people have had reviews that left a bad taste. But if you can follow these guidelines, you should be able to have a positive and productive experience.

How have your reviews gone?
What have you found that works or doesn’t work in code reviews? What special challenges have you encountered when involved in a more “formal” review process? Send us an e-mail with your suggestions and experiences.

 

Editor's Picks