Code Review Options

  • Article

There are many ways to conduct a code review.  Here are a few ways I've seen it done and the advantages and disadvantages of each.

Over-the-Shoulder Reviews

Walk over to someone's office or invite them to yours and walk them through the code.  This has the advantage that the author can talk the reviewer through the code and answer questions.  It has the disadvantage that it pressures the reviewer to hurry.  He won't want to admit he doesn't understand and will sign off on something without really validating it.  It also doesn't work for teams that are not co-located.

E-mail Based Reviews

This model requires some method of packaging the changes.  This is then e-mailed to another person for review.  The reviewer than then open the code in WinDiff or a similar tool and review just the deltas.  This tends to be efficient and allows the reviewer ample time to read and understand the code.  The downside is that the reviewer can take fail to prioritize the review correctly and it can sit around for a long time.

Centralized Reviews

Code reviews are sent (via an e-mail based system usually) to a centralized team of trained code reviewers.  These are usually the senior coders.  It has the advantage of helping to maintain high quality and uniformity in the reviews.  In a team trying to change coding styles or increase the quality of their code, this can be  a great way to do so.  The downside is that this team becomes a bottleneck.  If only a small portion of the team must review all the code, they can fall behind and reviews can back up.  Being part of the central review team can also be a drain on the most efficient coders.  Instead of writing code, they spend a large portion of their time reviewing code.

Decentralized Reviews

In this format, everyone in the team participates.  Any person can review any other person's code.  This form scales most easily with team size but can require some tools if it is to work on a large team.  Otherwise it becomes too hard to tell which people are participating and which are not.  The advantage of this form is that everyone is exposed to both sides.  This allows junior members to learn by reading code written by more experienced members.  The disadvantage is that the quality of the reviews is uneven.  Different members will have different abilities.  Over time this should even out as the team grows together.

Group Reviews

So far I've been speaking of reviews which involve only two people.  The reviewer and the reviewee.  There is another model in which a group gathers together in a room and the reviewer leads them through the code.  This model puts a lot of eyeballs on the code and is likely to find the issues better than any individual reviewer, but it also costs a lot of eyeballs.  This model uses the most manpower and is the least efficient reviewing model.  It suffers the same problems as the over-the-shoulder reviews except exacerbated.  The likelihood of people not being able to follow is high.  Additionally, the review will likely go slowly as people bring up questions about various points.  Finally, it may bog down into argument between the reviewers.  This model can be improved if participants review the code ahead of time and come prepared to only talk about their issues.  If they reviewers aren't reading the code in the meeting, it can be efficient.  Sometimes this is called a code inspection.

What We Use

There is no single right way to conduct reviews.  I won't pretend to proscribe a single manner of conducting them that will fit all organizations.  I will, however, tell you what I've found to work.  My team uses a distributed e-mail model.  We have a tool which will take the diff of the latest code in the repository and the code on your box and package it up.  The reviewee then sends mail to a mailing list asking for someone to review his or her code.  A reviewer volunteers and responds letting everyone know he is taking the review.  The reviewer then reads the code, sends his comments to the reviewee.  The reviewee then makes any required changes and eventually checks in the code.

Comments

  • Anonymous
    August 17, 2008
    There are a bunch of open source code reviewing systems now which you should check out which are a definite improvement over using just emails + patches.  I have been involved in Codestriker - http://codestriker.sf.net for many years, but there are plenty of other alternatives available now.

  • Anonymous
    September 22, 2008
    A couple of times a day I refresh from our source repository and eyeball most changes. If anything is glaringly obvious I'll try to address it via email or chat with the developer in question asap. I encourage the rest of the developers to do the same - if they are afraid to address the developer directly, they can always bring the issue to my attention. Sometimes waiting for the formal code review is too late - too much code has made it to the codebase and a refactor is not an option. If you do this from the get-go you can spot the trouble programmers and hopefully mend their ways before they do too much damage. You will also get a level of trust in what your programmers are capable of doing therefore reducing the amount of code checking you will feel you need to do.