When you think of a “code review,” you probably picture geeks huddled around a projector in a conference room criticizing a hapless developer as she walks through her code.

Besides putting the author in an unenviable position, this practice is also a potentially tremendous waste of time! Consider the traditional Formal Inspection, popularized by Michael Fagan [Fagan 1976], in which the focal point of the process is a two-hour meeting where four participants discuss the code.

Four people meeting for two hours is eight person-hours—an entire person-day of time! And that’s not counting the overhead for scheduling a conference room and waiting for stragglers to show. There’d better be a large benefit in future time saved to justify such a process. Or ice cream.

In Fagan’s theory the meeting is more than a convenience—it’s an absolute requirement. His rationale is that when you have four people working together you can create “synergy” (his word), where the whole is more than the sum of the parts. It’s almost as if there’s a fifth person in the room, finding bugs that no one could find alone, a virtual person Fagan calls the “phantom inspector.”

It’s a colorful notion, but is it true? Unfortunately, Fagan didn’t publish experiments testing this part of the theory; that would happen 17 years later [Votta 1993]. In 1993 Lawrence Votta measured the number of defects found before the meeting—when people went over the code themselves—and also the number of defects found during the meeting. His results are shown in Figure 18-4.

Figure 18-4. Votta demonstrated that inspection meetings contribute only an additional 4% to the number of defects already found by private code-readings

Of the defects found, 96% were identified just by looking at the code, not in the meeting. In retrospect this makes sense; the way you find subtle bugs is by spending alone-time with the code, not through a group walk-through where all your effort is spent just keeping up with the author as he explains each line.

To Fagan’s credit, the defects found in meetings were generally more subtle, which means they would have taken significant effort to find and fix later on. Still, the massive time requirement implies that in-person meetings aren’t worthwhile, except for the most vital source code.

Although meetings in general don’t uncover enough new bugs to warrant the time, they do serve another useful purpose: they identify false positives where a reviewer thought there was a bug but there really wasn’t. False-positives are an important drain on efficiency because they require developer time to fix and might create a true bug in the process.

In Votta’s study [Votta 1993], 25% of the so-called defects identified before the meeting turned out not to be defects at all, but rather a misunderstanding by the reviewer. And it’s not just Votta; other studies have shown false-positive rates between 15% and 30% [Conradi 2003][Jazayeri 1997][Kelly et al. 2003].

The question is: What is the penalty for “fixing” these false-positives? And then, is this penalty greater than the time penalty incurred by the meeting?

Generally if one person was confused, someone else is likely to be confused as well. Therefore, even if the “fix” is just adding a comment, that’s probably still a worthwhile use of time.

Finally, meeting together in the same room isn’t the only way to identify false-positives! So long as the author and reviewers can communicate at all—through email, chat, or a tool—they can discuss questionable defects, and most false-positives can be dispensed with. The point is that the physicality and immediacy of the meeting room isn’t necessary.

We’ve been assuming that two heads are better than one, but is that necessarily so?

Back to the analogy of editing your own writing, it’s true that you can’t see the errors on the screen, but if you print it out and view your text in a different medium, often the problems jump out. Or if you wait a week and come back to it, you can see a new problem. Could this apply to code reviews too?

This is a vital question because a self-check takes at most half the time of a code review involving another person.

A Cisco® study compared the results of 300 reviews where people reviewed their own work versus another 300 reviews where the author got a review without a self-check [Cohen 2006]. As demonstrated in Figure 18-5, self-check reviews had half the defect density of nonchecked reviews, indicating that people who double-check their work found half of the problems by themselves.

Figure 18-5. When authors proofed their own work, they found half of the defects that another reviewer would have found

Whether a simple “self-check” alone is sufficient for your own process is a matter of opinion. After all, plenty of defects were still found by other reviewers. But one thing is clear: it’s certainly an efficient use of time to self-check—and far better than nothing—since half the defects were found without any effort by external reviewers.