Our Thinking

Code Reviews: Benefits and Pitfalls

Posted by Dawson Kroeker on Apr 21, 2014 11:48:16 AM

Many software developers agree, at least in principle, that code reviews are a good idea. In fact, Jeff Atwood says:

…peer code reviews are the single biggest thing you can do to improve your code. [1]

There are many reasons why code reviews can be so effective. Here are a few.

Code Review Benefits

1. Code reviews find bugs

This is usually the primary goal when teams start using code reviews, and for good reason, as studies have shown formal code inspection to be very effective in detecting defects:

Most forms of testing average only about 30 to 35 percent in defect removal efficiency levels and seldom top 50 percent. Formal design and code inspections, on the other hand, often top 85 percent in defect removal efficiency and average about 65 percent. [2]

This has also been my personal experience. When I have given code reviews, I have found bugs on a consistent basis. Of course the same is true for when I receive a code review! Another set of eyes on my code often identifies bugs I previously did not see.

2. Code reviews improve code design

Good developers often have a much broader design in mind than their design for the code they are currently writing. They are anticipating what needs to be added in the future and how the code will change. When other developers work with their code, however, they may not be aware of the design behind the code. Code reviews really help here. When looking for a code reviewer, find someone that is familiar with the code you are working with. You may be surprised at what you learn during the review!

3. Code reviews improve ramp up time on new projects

Code reviews are a very effective way to introduce developers to a new project, especially when they are joining in the middle of the project. New developers will not be immediately familiar with the architecture of the code and how the components work together. It’s very difficult to explain everything before they begin coding. By giving a code review to a new developer, they have context for the information you are giving them and are more likely to understand what you are telling them.

Code Review Pitfalls

Unfortunately, there are also a number of pitfalls that can be very easy to fall into when implementing code reviews.

1. Massive check-ins

In a team environment it is vital that all programmers are checking their code into source control frequently, probably daily, in order to minimize merge conflicts and keep productivity high. Mandatory code reviews can work against this principle since developers must wait for a code review before checking in. If there is any perceived delay after submitting a piece of code for review, developers will quickly learn that it is easier to wait until the last minute to get a code review done so they don’t have to wait to continue coding. Not only does this undermine daily check-ins, but it pushes bug detection to much later in the development cycle, making bugs more difficult to deal with.

One way to prevent this problem is to allow developers to check in to the main branch without review, but have a gated release branch in which all code must be reviewed prior to being committed. This allows the code reviews to be done on a different stream from check-ins and developers can continue with frequent, hopefully daily, check-ins.

2. Rubber stamp code reviews

When code reviews are enforced as mandatory, developers may acquire the habit of “rubber stamp” code reviews. In these reviews, all value is lost as the code reviewer does not really understand the code; they are simply there to fulfil the accepted process. This may happen if the reviewer feels the code author is a more experienced developer, or if they are nervous of offending the author.

To prevent this, developers should select a code reviewer they feel understands the code under development and would be capable of writing the solution themselves. Reviewers should understand that by doing a code review, they are sharing the responsibility for the code. If the code breaks, they are just as responsible as the original author.

3. Unnecessarily slow development

The code review process can be slow – the developer:

  1. Completes coding a piece of work.
  2. Requests code review.
  3. Waits for code review.
  4. Receives code review and make changes as required.
  5. Repeats steps 1-4 until no more changes are required.

Often the biggest challenge is the waiting in step #3. If the developer wants to continue working, they need to do a context switch and start working on a different task. Or they continue working on the same task, but then need to undo these changes so they can incorporate any changes from the review and check them in. If code reviews are required for the simplest of changes, this can add a very high percentage of overhead for each check-in.

Using a gated release branch as suggested in the first pitfall may also remove some of these costly context switches. Alternatively, consider using pair programming in the coding cycle. Pair programming is effectually a continuous code review, but doesn’t require the repetitive waiting cycle shown above. This method is also a great way to bring less experienced coders up to speed on best practices and architecture decisions throughout the code base.

Code reviews are an excellent way to improve code quality and design. Unfortunately, in practice they can lead to pitfalls causing frustration and delay. Mandatory code reviews may cause more hassle than they are worth, but regular reviews should still be adopted by quality conscience developers.

______________________________________

[1] Attwood, Jeff. (2006, Jan 21). Code Reviews: Just Do It. Retrieved from http://blog.codinghorror.com/code-reviews-just-do-it/

[2] Jones, Capers. (2008, June). Measuring Defect Potentials and Defect Removal Efficiency. Retrieved from http://www.crosstalkonline.org/storage/issue-archives/2008/200806/200806-0-Issue.pdf

Topics: Application Development, Quality Assurance

Our Thinking - The Online Blog is a source for insights, resources, best practices, and other useful content from our multi-disciplinary team of Onliners.

Subscribe to Blog Updates