Code Reviews: 2.5 Hours Per Review

In a code review, the team examines a sample of code and fixes any defects in it. A defect is a block of code that does not function as the programmer intended, or that is not incorrect but could be improved (for example, it could be made more readable or its performance could be improved).

Implementing code reviews is an effective way to help the team build better software. In addition to helping teams find and fix bugs, code reviews are useful for cross-training programmers on the code being reviewed and helping junior developers learn new programming techniques. Most importantly, developers tend to write better code when they know someone else will read it later.

The first task in a code review is to select the sample of code to be inspected. It's impossible to review every line of code, so the programmers need to be selective about which portion of the code gets reviewed. If the right code samples are selected for review, then code reviews can be effective. It turns out that in most projects, a large number of defects tend to be concentrated in relatively small pockets of code. If the code is selected well, then the review easily saves the team far more time than it costs by helping the team catch defects that, if left in the software, would have required much more time to track down and fix later on.

It's not hard for the lead developer to select the right code samples for review. Code that's a good candidate for review may implement a tricky algorithm, use a difficult API or object interface, require specific expertise to maintain, or may employ a new programming technique. It is especially helpful to select a code sample in a high-risk part of the software in which any defects will be especially catastrophic—this is useful not just because that code is likely to have more defects, but also because more people will be able to maintain it down the line. When there is a large repair that requires extensive changes to many areas of the software, there is a high risk of introducing defects—this code makes a good candidate as well.

To prepare for the review, the lead distributes a printed copy of the selected code (with line numbers) to each team member. The team members each spend half an hour individually reading through (and, if possible, executing) the code sample. They do their best to figure out whether the code really does what the author intended it to do. They should look for problems with accuracy, maintainability, reliability, robustness, security, scalability, reusability, or efficiency. Any of these problems should be considered a defect. Each team member tries to spot as many defects as possible, marking them down on the printed copy.

After the individual preparation, the team leader gathers everyone for the review meeting. The code review session starts with the lead developer reading the first small block in the code sample aloud. He doesn't literally read the commands in the code; he simply gives a brief description (about one sentence) of the purpose of that block. If anyone (including the lead) does not understand what the code does or disagrees with the interpretation, the author of the code explains what it is the code is supposed to accomplish. Sometimes a team member can suggest a better, more self-explanatory way to accomplish the same thing; often it is simply a matter of explaining the purpose of the code to the person who raised the issue.

Team members should then discuss any defects found in that block of code. This is where the lead must act as a meeting moderator. If anyone found a defect, the lead should decide whether or not the team can come up with a way to fix it on the spot. If he decides they can, they do that. If not, he notes it as an open issue for the programmer to fix later. In either case, the lead adds a row to a spreadsheet that contains the review log. This spreadsheet should have one row per defect found, where each row lists the line number that contains the defect, the person who identified it, and a description of how to resolve it (or an indication that the issue was left open). At the top of the log, the moderator should note when the meeting took place and identify which block of code was reviewed.

The meeting should take no more than two hours. If it lasts longer than that, then a shorter code sample should be selected for future code reviews. Once the meeting is over, the lead should e-mail the log to the rest of the team and assign defect repairs to whoever is responsible for that block of code. Once the defects are repaired, he should review the updated code to make sure that it was repaired properly.