The Case for Peer Review

[article]
Summary:

The $1 billion bug and why no one talks about peer code review.

 It was only supposed to take an hour.

The bad news was that we had a stack of customer complaints. The latest release had a nasty bug that slipped through QA. The good news was that some of those complaints included descriptions of the problem - an unexpected error dialog box - and one report had an attached log file. We just had to reproduce the problem using the log and add this case to the unit tests. Turn around a quick release from the stable branch and we're golden.

Of course that's not how it turned out.  We followed the steps from the log and everything worked fine.  QA couldn't reproduce the problem either.  Then it turned out the error dialog was a red herring - the real error happened long before the dialog popped up, somewhere deep in the code.

A week later with two developers on the task we finally discovered the cause of the problem.  Once we saw the code it was painfully obvious - a certain subroutine didn't check for invalid input.  By the time we got the fix out we had twenty more complaints.  One potential customer that was trialing the product was never heard from again.

All over a simple bug.  Even a cursory glance over the source code would have prevented the wasted time and lost customers.

The worst part is that this isn't an isolated incident.  It happens in all development shops.  The good news?  A policy of peer code review can stop these problems at the earliest stages, before they reach the customer, before it gets expensive.

The case for review: Find bugs early & often

One of our customers set out to test exactly how much money the company would have saved had they used peer review in a certain three-month, 10,000-line project with 10 developers.  They tracked how many bugs were found by QA and customers in the subsequent six months.  Then they went back and had another group of developers peer-review the code in question. Using metrics from previous releases of this project they knew the average cost of fixing a defect at each phase of development, so they were able to measure directly how much money they would have saved.

The result: Code review would have saved half the cost of fixing the bugs.  Plus they would have found 162 additional bugs.

Examine the following charts for the raw data.  (Critical readers might take issue with the given dollar amounts.  These numbers came from the customer and of course they are arguable and differ between organizations.  However, as long as you agree that "bugs found later in the process cost more than bugs found earlier" the argument for code review still holds.)

Saving $150k: A real-world case study

jc0806-2


jc0806-1jpg

Why is the effect of code review so dramatic?  A lack of collaboration in the development phase may be the culprit.

With requirements and design you always have meetings.  You bring in input from customers, managers, developers, and QA and synthesize a result.  You do this because mistakes in requirements or architecture are expensive, possibly leading to lost sales.  You debate the relative priorities, difficulty, and long-term merits of your choices.

Not so with when actually writing the source code.  Individual developers type away at the tasks assigned to them.  Collaboration is limited to occasional whiteboard drawings and a few shared interfaces.  No one is catching the obvious bugs; no one is making sure the documentation matches the code.

Peer code review adds back the collaborative element to this phase of the software development process.

Consider this: Nothing is commercially published without corrections from several professional editors.  Find the acknowledgments in any book and you'll find reviewers who helped "remove defects."  No matter how smart or diligent the author, the review process is necessary to produce a high-quality work.  (And even then, what author hasn't found five more errors after seeing the first edition?)

Why do we think it's any different in software development?  Why should we expect our developers to write pages of detailed code (and prose) without mistakes?

We shouldn't.  If review works with novels and software design it can also work when writing code.  Peer code review adds a much-needed collaborative element to the development phase of the software development process.

The $1 billion bug

In 2005, Adobe attributed $1 billion in revenue to their stronghold on the PDF format (see Adobe Systems Incorporated Letter to Stockholders FY 2005).

Why is PDF worth $1 billion?  Because it's the one format that everyone can view and print (see same document).  It just works.  If it loses that status, Adobe loses the edifice built on that format, to which the fiscal year 2005 income statement attributes $1 billion.

Now imagine you are a development manager for Acrobat Reader, Windows Edition.  The next major release is due in 9 months and you are responsible for adding five new features.  You know how much is riding on Reader and how much revenue - and jobs - depends on its continued success.

So now the question: Which of those five features is so compelling, it would be worth introducing a crash-bug in Reader just to have that feature?

Answer: None!

Nothing is worth losing your position in the industry.  But you still must implement new features to keep the technology fresh and competition at bay.  You must employ every possible technique in your development process to ensure that no bugs get introduced.

Only code review will ensure that this code base - already over ten years old - remains maintainable for the next ten.  Only code review will ensure that new hires don't make mistakes that veterans would avoid.  Static code analysis and black-box QA will not do this for you.  And every defect found in code review is another bug that might have gotten through QA and into the hands of a customer.

This doesn't mean they implement code review no matter what the costs; developer time is still an expensive commodity.  It does mean that they're taking the time to understand this process which, if implemented properly, is a proven method for significantly reducing the number of delivered bugs, keeping code maintainable, and getting new hires productive quickly and safely.

But you don't need to have $1 billion at stake to be interested in code quality and maintainability.  Delivering bugs to QA costs money; delivering bugs to customers costs a lot of money and loss of goodwill.

But if code review works this well, why don't more people talk about it?  Is anyone really doing it?

Why code review is a secret

In 1991, OOP was the Next Big Thing.  But strangely, at OOPSLA there were precious few papers, light on content, and yet the attendees admitted to each other in hallway talk that their companies were fervently using the new techniques and gaining significant improvements in code reusability and in breaking down complex systems.

So why weren't they talking publicly?  Because the development groups that truly understood the techniques of OOP had a competitive advantage.  OOP was new and everyone was learning empirically what worked and what didn't; why give up that hard-earned knowledge to your competitors?

A successfully-implemented code review process is a competitive advantage.  No one wants to give away the secret of how to release fewer defects efficiently.

When we got started no one was talking about code review in the press, so we didn't think many people were doing it.  But our experience has made it clear that peer code review is widespread at companies who are serious about code quality.

But the techniques are still a secret!  Peer code review has the potential to take too much time to be worth the gain in bug-fixing, code maintainability, or in mentoring new developers.  The techniques that provide the benefits of peer code review while mitigating the pitfalls and managing developers' time are competitive advantages that no one wants to reveal.

Unfortunately for these successful software development organizations, we make a living making lightweight, agile code review accessible and efficient for everyone.  And that's the subject of this CM Crossroads Article Series.

I'm interested.  Gimme the details!

So code review works, but what if developers waste too much time doing it?  What if the social ramifications of personal critiquing ruin morale?  How can review be implemented in a measurable way so you can identify process problems?

In this article series we will cover case studies of review in the real world and show which conclusions you can draw from them (and which you can't).  We will give our own case study of 2500 reviews.  We will give pro's and con's for the five most common types of review.  We will explain how to take advantage of the positive social and personal aspects of review as well as ways managers can mitigate negative emotions that can arise.  We will explain how to implement a review within a CMMI/PSP/TSP context.  We will give specific advice on how to construct a peer review process that meets specific goals.  Finally, we will describe a tool that our customers have used to make certain kinds of reviews as painless and efficient as possible.

Code review can be practical, efficient, and even fun.  Stay tuned!

(Special thanks to Steven Teleki (http://steven.teleki.net/) for his insights into the parallel between the rise of OOP and the modern rise of peer code review.)

About the author

CMCrossroads is a TechWell community.

Through conferences, training, consulting, and online resources, TechWell helps you develop and deliver great software every day.