The Fall 2009 Perforce newsletter featured Part 1 of this series, in which I talked about different methods of code review you can employ to find bugs sooner in the product lifecycle.
Once you've chosen the right method for your team, then what? What's the most efficient way to do code review? How do you get started?
It's both common knowledge and common sense that peer code review—in which software developers review each other's code before releasing software to QA—identifies bugs, encourages collaboration, and keeps code more maintainable. You wouldn't write a book or even an article without at least one editor to check it for errors...why would you even consider writing something as important as code without an extra pair of eyeballs to provide a sanity check?
But like any process, your approach to code review matters; some practices are better than others. The stakes are high: if it costs X to find and fix a bug during the development phase, industry data indicates that it typically costs 8-12X to eradicate that bug during the Quality Assurance process and 30-100X to expunge it after the software has gone out to customers! And let's not even talk about the damage bugs do to your company's reputation. So it's highly cost-effective to implement the most efficient, potent process possible. Fortunately, we can draw from a body of best practices research to help guide us down the right path.
World's Largest Code Review Case Study: Cisco, Perforce, & Code Review Tool
A few years go, Cisco Systems worked with my company, Smart Bear Software, to conduct the world's largest published case study of peer code review, spanning 2,500 reviews of 3.2 million lines of code over a 10-month period. We used Perforce and our Code Collaborator code review tool in the study, which was designed to examine how programmers review code and how to improve the process. Our conclusions were aided by metrics automatically collected from Code Collaborator, which provides review-level and summary-level reporting.
Figure 1: Inspection effectiveness falls off when greater than 500 lines of code are under review.
The case study yielded useful process implications and best practices for any kind of review.
Review for No Longer than 60-90 Minutes
Reviews should not last longer than 60-90 minutes. After that amount of time, programmers get tired, lose focus, and performance starts to drop off.
This conclusion is well-supported by evidence from many other studies besides our own. In fact, it's generally known that when people engage in any activity requiring concentrated effort, performance starts dropping off after 60-90 minutes. On the flip side, you should always spend at least five minutes reviewing code—even if it's just one line. Often a single line can have consequences throughout the system, and it's worth the five minutes to think through the possible effects a change can have.
Optimal Velocity: 300-500 LOC Per Hour or Less
Take your time with code review. Faster is not better. Our case study showed that you achieve optimal results at an inspection rate of less than 300-500 lines of code (LOC) per hour. Reviewing faster than 400-500 LOC/hour causes a severe drop-off in effectiveness. And at rates above 1000 LOC/hour, you can probably conclude that the reviewer isn't actually looking at the code at all.
Review Fewer than 200-400 LOC at Once
The Cisco code review study showed that for optimal effectiveness, developers should review fewer than 200-400 LOC at a time. Beyond that, your ability to find defects diminishes. At this rate, with the review spread over no more than 60-90 minutes, you should get a 70-90% yield; in other words, if 10 defects existed, you'd find 7-9 of them.
Figure 2: Defect density dramatically decreases when the number of lines of inspection goes above 200, and is almost zero after 400.
Author Preparation Matters—A Lot
It turns out that author preparation makes a huge difference in code quality and review efficiency. To prepare for a review, authors annotate their source code before the review begins. Annotations guide the reviewer through the changes, showing which files to look at first and defending the reason and methods behind each code modification. These notes are not comments in the code, but rather comments given to other reviewers.
Author preparation has a hidden benefit beyond helping reviewers navigate: often, as authors are re-thinking and explaining their code, they discover their own defects before the review even begins, thus making the review itself more efficient. Of course, this kind of self-review is a good idea even if your team doesn't do code reviews!
Getting Started with Code Review: Small Scope Yields Most Impactful Results
So now you've learned some best practices...what's the best way get started?
The optimal way to begin is at a small scale, by reviewing only a handful of files. Don't plan to review all of your code at the outset. Instead, these suggestions for limiting review scope will help you achieve maximum results in minimal time. Choose however many of them make sense for your team.
- Review changes to the stable branch only
- Review changes to the core module that all other code depends on
- Review changes to the "top 10 scariest files" as voted by the developers
- Review only unit tests—if they are complete and not over-specified, any bugs or other problems can be fixed safely later
- Review only changes, not entire files
- Review code whenever developers think it's necessary, such as when they have concerns about a section of code or when they're working on code they didn't write and may not fully understand
When programmers see dramatic results that improve their code but require little effort on their party, they buy into the process! Implementing code review without team buy-in almost pre-destines the process to fail. Starting small helps convince skeptics that code review is worth everyone's time.
Try It and See
Code review will significantly improve the quality of your software, and Perforce provides a variety of features and workflow that support streamlined code reviews.
"Still...is it really worth our time?" Good question. The easiest way to find out is to try it and measure the actual results for your team. Get your team to consent to try code review for one week and to agree to spend a total of 2-2.5 hours per programmer. Then follow these steps:
- Establish the code review method you're going to use. Tool-supported, Over-the-Shoulder, or Email Pass-around methods are recommended for this evaluation week (see Part 1 of this series for pros and cons of each method). I recommend choosing one of these methods because they're easiest to conduct and track.
- Have everyone do code reviews for 20-30 minutes per day for that one week.
- During each review, capture two simple metrics: Number of Bugs Found and Time Spent.
- Calculate the rate at which your team finds bugs. Divide the total time spent on reviews by the number of defects found. For example: 25 hours spent divided by 100 defects found yields 0.25 hour (15 min) spent to find one defect.
Consider how more much time and money that defect would cost to fix if it slipped through to QA (typically 8-12X), or worse, to customers (30-100X!). Now discuss the results with the team and let the group decide...is it worth it?
If the team doesn't see the benefits at that point, then code review is not for you. But chances are they will, and then you'll have the buy-in you need to conduct successful code reviews. And now you know how to do them right.
Jason Cohen is the founder of Smart Bear Software. In conjunction with Cisco Systems, Jason conducted the world's largest study of lightweight peer code review and published the findings in his book, Best Kept Secrets of Peer Code Review. To request a free copy of the book, go to www.CodeReviewBook.com
All trademarks and registered trademarks are property of their respective owners.