Crash Course in Lightweight Code Review

Get results in a week

ByJason Cohen
March 03, 2009
URL:http://www.ddj.com/architecture-and-design/215800147

Jason Cohen is the Founder of Smart Bear Software and author of Best Kept Secrets of Peer Code Review.

Most programmers agree that another pair of eyes on your code willuncover bugs and disseminate knowledge across development teams. Butmany also recognize that peer review can waste a lot of time.

So how do you get started in such a way that you 1) don't waste time, 2)match the process to your team and your goals, and 3) have a clear wayto evaluate results? So many code review techniques exist, and each withpros and cons, so which are right for your team? Even if you'reunwilling to spend the time to review all your code, perhaps spending alittle time reviewing a specific subset would be worthwhile.

The only way to know is to try it for yourself. Use these tips to simplify, expedite, and measure the process.

What Is Code Review and How Do You Do It?

First, let's make sure we're on the same page. What is "code review?"The answer varies depending on whom you ask, and when; code review haschanged dramatically in recent years.

Thirty years ago, if you wanted a well-known and measurable process that delivered results, your only choice was the dreaded Formal Inspection-- a seven-phase, multi-meeting, paperwork-heavy system popularized byMichael Fagan at IBM. Properly done, it takes over ten person-hours toreview at most 200 lines of code -- a sluggish rate that makes thissystem impractical (not to mention excruciating) even if it does uncoverbugs.

In the past 10 years a variety of evidence [1][2][3][4] has shown thatmany of the components of the traditional "heavyweight" inspectionprocess are inefficient or even unnecessary. "Lightweight" modern codereview methods include:

• Over-the-Shoulder: The reviewer physically looks over the author's shoulder and is walked through the code.
  • Pros: Easy to implement, no tools required.
  • Cons: Interrupts reviewers, hard to know which code is reviewed, impossible to measure and track. Requires coordinating with someone else's schedule.

• Email Pass-Around: Code differences are emailed automatically by the version control system. Reviewers respond to changes when they have time.
  • Pros: Easy to set up, nothing to purchase, works remotely.
  • Cons: No tracking -- don't know if found defects were fixed, often reviews never happen because everyone assumes someone else looked at it, no metrics; reviews "fizzle out" instead of finishing. Hard to follow conversations as emails get replied to and forwarded among several people.

• Tool-Based Review: Use a development tool specifically designed to assist in code review. A variety of open-source and commercial tools exist.
  • Pros: Removes most of the busywork associated with file-collection, discussion/defect management, and metrics/reporting. Tracks reviews and defects.
  • Cons: Have to learn and integrate another tool, can cost money.

• Pair-Programming: Two developers at the same keyboard and monitor working on the code together.
  • Pros: Deep level of review; teams know review is happening but it's difficult to track.
  • Cons: Large time investment, some developers dislike it. Requires coordinating with someone else's schedule.

Pick a method that most closely matches the existing culture of theteam. Use the pros, cons and amount of time you have to select one andstick with it for one week before you start to make changes.

Getting Started: Baby Steps

It's difficult to introduce a process that involves a large disruption:projects get behind, people rebel, and the process fails -- regardlessof potential future efficiency.

It's even harder when there's push-back from developers who aren't yetconvinced the process makes sense. This effect is common in code reviewbecause people recall previous bad experiences with time-sucking formalinspections or with review processes that were designed with more rulesthan thought. There's also the worry of hurting people's feelings.

So the best way to introduce code review is at a small scale, with only ahandful of files, incrementally, and with full transparency to theteam. Taking baby steps is especially effective against those who say"We cannot change our process right now."

Given this, you'll want to select a style of code review that islightweight and easy yet provides transparency by tracking things like"number of defects found" and "time spent in review." Any of themethods listed above will work; some are easier to get going but requiremore manual work to collect metrics. A good compromise as you'regetting started is to use a code review tool for free, either bytrialing a commercial tool or using an open-source tool. That way youget the benefit of structured guidance through the review but you can doit without dipping into the budget.

Low-hanging Fruit: Review What Matters Most

Besides the social and training benefits of introducing a small changeto your process at a time, you may find that you can reap significantbenefits even if you never get to the point where you're reviewing everyline of code. If it turns out to be valuable only in specificcircumstances, that's okay. Not every process needs to cover every filein every project over all time and all developers.

So if you're only going to do a little bit of review, what should you review to have the greatest impact?

Here are some ideas:

• Review changes to the stable branch
• Review changes to the core module -- the one all other code depends on
• Review changes to the "Top 10 Scariest Files" as voted by the developers
• Review only unit tests
• Review only incremental changes, not entire files
• Review code whenever a developer feels it's necessary

Once you get comfortable you can cast your net to a wider set of code if it makes sense.

By then you'll have a good idea of what you're getting from review andhow much time it's taking, so you'll have the information you need todecide exactly how to expand the process.

Not All Bugs are Created Equal

It's worth mentioning that not all bugs are easy to find in code review.GUI problems are a great example. GUI code is often generatedautomatically and it's almost impossible to read the code and know ifthe GUI makes sense. In QA, you can instantly spot visual flaws, andmechanical problems are usually straightforward as well. Similarly,fixing bugs in GUIs is usually a simple, fast matter.

Clearly the type of bug found in code review matters to your efficiencyand effectiveness calculations. Track the "category" of bug so you candetermine efficiency by type. Example types are:

• Algorithm
• Build
• Data Access
• Documentation
• Error-Handling
• Interface
• Maintainability
• Performance
• Robustness
• Style
• Testing
• User Interface

This way you can empirically determine where your reviews are efficient and where they're a waste of time.

Measure Success

Code review takes up valuable developer hours, so it'd better be worth it!

Researchers have proposed many ways of measuring the productiveness andefficiency of code review, usually centered on the number of defectsuncovered per thousand lines of code. But the clearest way is to usethe two things that matter most: time and bugs.

The easiest way to visual this is: "How much time does it take us tofind and fix one bug?" I call this "Time per Bug Fix." The formula issimply: (Total Time Spent ) / ( Number of Defects Found ).

This metric is clear and tangible; you can easily understand what itmeans and why it's interesting. It's also easy to compare to otherprocesses, even if you're just estimating.

For example, how long does it take to find and fix a bug found in QA? AQA person has to run through some code, discover a problem, create atrouble ticket, and write down how to reproduce the problem. Then, adeveloper has to go find the bug - often the evidence in QA is an errordialog that doesn't help you understand the root cause of the problem -then fix it. QA then has to verify the fix before it can be consideredfinished.

A typical value for Time per Bug Fix for a lightweight peer reviewprocess is 5 to 15 minutes. The QA cycle described above is surely muchlonger than that. And let's not even talk about the damage a bug can do(in both costs and reputation) if it escapes QA and gets intocustomers' hands.

You can get your own number in only a week. Have everyone review codefor twenty minutes a day for five days and you'll have enough data for ameaningful result.

Start Today

With a sufficiently lightweight peer code review process, focused on aspecific subset of your code, designed to look for a specific set ofproblems, there's no doubt you'll able to find and fix defects fasterand more easily than any other method.

Now you have the background to test and build a process that gets you there, so there's no excuse! Get started today.

References

[1] Lawrence G. Votta, Jr., Does every inspection need a meeting?, Proceedings of the 1st ACM SIGSOFT symposium on Foundations of software engineering, p.107-114, December 08-10, 1993, Los Angeles, California, United States

[2] Reidar Conradi, Parastoo Mohagheghi, Tayyaba Arif, Lars Christian Hegde, Geir Arne Bunde, and Anders Pedersen; Object-Oriented Reading Techniques for Inspection of UML Models - An Industrial Experiment. In European Conference on Object-Oriented Programming ECOOP'03. Springer-Verlag, Darmstadt, Germany, pages 483-501

[3] Kelly, D. and Shepard, T. 2003. An experiment to investigate interacting versus nominal groups in software inspection. In Proceedings of the 2003 Conference of the Centre For Advanced Studies on Collaborative Research (Toronto, Ontario, Canada, October 06 - 09, 2003). IBM Centre for Advanced Studies Conference. IBM Press, 122-134.

[4] In Proceedingsof the 6th European Conference Held Jointly with the 5th ACM SIGSOFTinternational Symposium on Foundations of Software Engineering(Zurich, Switzerland, September 22 - 25, 1997). M. Jazayeri and H.Schauer, Eds. Foundations of Software Engineering. Springer-Verlag NewYork, New York, NY, 294-309.