Why Do Code Reviews?If code reviews are so painful, why do we do them? Ultimately, there are only two reasons. First, they improve overall code quality, and second, they improve developers' skills. Code reviews improve overall quality because they provide a venue to evaluate code and only code. They provide a defined opportunity to consider code structure, layout, style conformance, problem-solving methods and other aspects of the work. (You can see more on my code review checklist, although details will vary based on your specific environment and team.) Also, code reviews help developers improve their skills by providing a feedback loop. They're a way to look at an individual developer's work, acknowledge their great ideas, and identify specific areas for improvement. Most software products are the result of a team working together, and the code review is one of the few times the group has a chance to look at individual work out of the context of the overall product. System tests evaluate a product as a whole: They look at the work of one or more teams. Unit tests evaluate a piece of the product: They look at the work of multiple people over time. Code reviews look specifically at the work of one person. Because they're so focused on the individual, code reviews can be scary. They can turn into "pick fests" where reviewers gang up on the developer. Taken to extremes, this creates an extremely toxic environment. To keep the code review effective, you have to turn it into a positive experience. To do that, they have to be egalitarian — everyone's code must be subject to review. In addition, make sure code reviews are about the code.
Types of Code ReviewsCode reviews come in several flavors: ad hoc, architect-driven, and peer. Any of these can be done at any point in the software development process — before the code is checked in, before the build is done, after the initial testing process or even after release. Ad hoc code reviews are tactical reviews conducted when the team — or someone on the team — feels they're necessary. A team might conduct a code review when changing a core algorithm, or for a new developer just coming up to speed. They're driven by circumstances and not by a defined pattern or trigger. For a team that doesn't do code reviews regularly, ad hoc reviews are often an easy way to start. The major risk of ad hoc reviews is that they are prone to become a punishment mechanism. It's very easy to use an ad hoc review to gang up on a developer. Imagine a scenario in which one person's code "magically" comes up for review far more than others'. That's a recipe for creating a seriously unhappy developer. Architect-driven reviews are the traditional approach. In this model, an architect or senior developer reviews the code. The emphasis is on oversight and senior technical resources educating junior technologists. In some cases, the reviewer is the team lead. In others, the reviewer (or review committee) is a separate team, often an oversight or "office of the CTO" committee. These are the code reviews everyone dreads. They tend to go through large swaths of code in meetings that last hours, then hand down findings without discussion. I avoid this model as much as possible for general code reviews. However, it can be useful when conducting a highly targeted review that looks only at specific aspects of the code. For example, an architecture-only review. Peer code reviews -- where team members review each other's code — are becoming increasingly common. They cover all aspects of the code and emphasize discussion over review. The idea is that explaining your code to another team member — even a more junior one — is a learning experience for both of you and can help each person to identify flaws in the code. Pair programming is a form of real-time review. The downside of peer code reviews is that they can end up being a case of the blind leading the blind. To make them work, at least one participant has to be senior enough to have good instincts.
Facilitating Code ReviewsConducting an effective code review takes some set-up. It doesn't have to be terribly involved, but a little effort can make everything a lot easier. In your review, consider the following:
- Source code branching.
- Code commenting systems.
- Atomic units of review.
- Static code tools.
- Work tracking system.
Next week I'll look at source code management tools, and also how the branching or commit mechanisms you use affect how easy -- or difficult -- it is to isolate code for review.