The Source for Java Technology Collaboration
User: Password:
Register | Login help    

Search

Online Books:
java.net on MarkMail:


Best Kept Secrets of Peer Code Review

Posted by gsporar on October 1, 2006 at 12:20 PM PDT

Long ago, back when green screens and green-bar paper roamed across the software development landscape, I was a young software developer at a startup software company. There was a development group that I worked with that held periodic code reviews. These were conducted as lengthy meetings with the entire team gathered around a conference room table. Everyone had a printout of the code (written in COBOL), and they were expected to have studied it closely before the meeting. The team member whose code was being reviewed would lead the meeting. I sat in on one of those meetings and it appeared to me that the return on investment (ROI) was pretty low. The leader of the meeting would say things such as, "And on line 147 I'm initializing the control break variable." The collective response was usually silence.

I suppose they occasionally found bugs using this approach, but it sure seemed like an expensive, heavyweight process. Years later I worked in an environment where we used a much lighter weight code review technique. It was a casual process where at any moment the developer who wanted to commit code would wander down the hall and find someone to do a review. The developer would explain the code changes and the reviewer would watch as the developer stepped through the code displaying the differences between the current version and the proposed changes. The cost of this lightweight approach was much lower, but I was always bothered by two things: the developer had complete control over the process and the reviewer had no prior access to the changes.

Two different environments with very different approaches to doing code reviews. The one thing they had in common was that there was never any attempt made to calculate the return on investment. I suspect it was low in both cases. Low ROI is perhaps the main reason that many software developers avoid code reviews.

Jason Cohen, the author of Best Kept Secrets of Peer Code Review argues that reviewing code can have an excellent ROI if done correctly. The company he runs, Smart Bear Software, is currently giving away copies of his book for free, including shipping. At that price and considering that it's a quick (164 pages) and painless read, you cannot go wrong. Naturally, there is a bit of a catch. Smart Bear Software sells tools to help you conduct code reviews. So the book is basically a way for them to get the word out on practices that they not only believe in, but also have built a part of their business around. Note: I have no vested interest in Smart Bear Software (Updated: That was true when I originally wrote it in 2006, but in November 2008 I became an employee of Smart Bear Software) - I just liked the book.

The book is much more than just a pitch for Smart Bear's products. It includes a brief history of the research that has been done on reviewing (or inspecting) code, starting with the work of Michael Fagan. There is considerable coverage of more recent research, including a really interesting looking study of code reviewer eye movement.

The best thing about the book is that the research is distilled into practical advice and guidelines for doing code reviews. Specific suggestions are given for how to conduct code reviews that deliver the highest possible ROI, including information on which metrics to use for calculating ROI.

There were some things that seemed missing though. For example, in describing the checklists that developers should use to self-inspect their code, it seemed like for many programming langugages some of the items on the list could have been automated. The lack of a mention left me wondering if maybe the author considers static analysis tools and the like to be a bad idea. And there are some rough spots in the text: there are vague references in one chapter to content in some other chapter without the specific chapter number, name, or page number. Finally, while most of the book is carefully footnoted, there is one chapter with no footnotes and several statements such as: "In 2002 it was reported that the average career in high-tech lasts 8 years." Okay, I might be willing to believe that, but only if I know the source.

In the end, though, those complaints are minor. The book is a worthwhile read. You can pick up some valuable tips regardless of how you do code reviews.

Comments
Comments are listed in date ascending order (oldest first)