Sunday, June 8, 2008

On code reviews

One of the most valuable lessons I've learned from working at Google is the importance of code reviews. There was no mention of code reviews in school or at my previous employers, but now I can't imagine developing without it. Code reviews come in many forms but for my current project it comes down to one thing: Every piece of code must be okay'ed by at least one coder other than the code's writer before it gets checked in. That's it.

Here's a typical cycle:

1) Coder A writes code to add a small feature (or fixes a bug, or whatever).
2) Coder A shows diffs for this change to coder B (and possibly C, D, E, etc.)
3) Coder B reads through the code and notices some small logic bugs, a few style issues, and raises some other concerns.
4) Coder A fixes the issues that B brought up and responds to B's concerns.
5) Repeat 2-4 as many times as required.
6) Coder B is now happy and says so - in writing (e.g., via email)
7) Coder A checks in the change.

Sound painful? Sometimes it is. But the value added by this cycle is quite remarkable. It's extremely valuable to have people inspect the code before it gets in for many reasons, including:

1) Small logic bugs resulting from the same person staring at the same code for several minutes or hours are often caught quickly by a second set of eyes.

2) At least two people see every piece of code checked in, spreading the knowledge throughout the team and reducing the likelihood of ending up with code that only one person understands and/or can modify.

3) People are more cautious about what they check in. With code reviews, most developers do additional work to clean up, comment, and optimize their code before requesting a review in order to minimize the number of review iterations.

4) The blame can be shared among the team when things go wrong. This may seem like a small deal, but I think it's very important. No matter how careful people are, imperfect code will continue to get checked in. Without code reviews, it's easy to place the blame and this can cause animosity in the team. With code reviews, multiple people are involved in imperfect code getting checked in. This means that not only is it in everyone's best interest to make sure the best possible code gets checked in, but also mistakes become something that the team can improve on instead of just "coder A screwed up".

Code reviews add overhead, there's no question about it. But the value of code reviews is comparable to unit tests and other magic tools of software development productivity, so if you aren't using them, you should.


Brian Pearson said...

Good post Derek. I'll be using your arguments to try and convince others at my job to take a similar approach.

Cai said...

Thanks for writing this.