44   Review Code

“Users make great testers. Don’t worry—if it’s wrong, they’ll tell us eventually.”

The best time to find problems in code is as soon as the code is written. If you let code sit and rot for a while, it won’t smell any prettier.

As Capers Jones points out, code reviews are probably the best single way to locate and solve problems. Unfortunately, it’s sometimes hard to convince managers and developers to use them.

Managers worry about the time it takes to review the code. They don’t want their team to quit coding and get into lengthy code review meetings. Developers fear code reviews; they feel threatened by others looking at their code. It affects their egos. They are worried about being emotionally beaten up.

On projects where your authors have practiced code reviews, the results have been remarkable.

Venkat recently worked on a very aggressively scheduled project with some less experienced developers. They were able to deliver top-quality and stable code by using rigorous code reviews. When developers on the team completed their coding and testing of a task, the code was thoroughly reviewed by another developer before it was even checked in to version control.

Quite a few problems were fixed in this process. Oh, and code reviews are not just for code written by junior developers—you should perform code reviews on the code written by every developer in the team, regardless of their experience.

So how do you review code? You can choose from a couple of different basic styles.

The all-nighter.

You could hold a one-night-a-month monster code review session, bring your entire team together, and order pizza. But this may not be the most effective way to hold code reviews (and doesn’t sound particularly agile). Large review teams tend to get into lengthy and extended discussions. Such a broad review may not only be unnecessary, but it might even be detrimental to progress. We don’t recommend this style.

The pick-up game.

As soon as some code has been written, compiled, tested, and is ready for check-in, the code is picked up for review by another developer. These “commit reviews” are designed to be a quick, low-ceremony way to make sure the code is acceptable before checking it in. In order to eliminate any behavioral ruts, try to rotate through developers. For instance, if Jane reviewed Joey’s code last time, ask Mark to review it this time. This can be a very effective technique.^[41]

Pair programming.

In Extreme Programming, you never code alone. Code is always written in pairs: one person sits at the keyboard (the driver), and one person sits back and acts as navigator. Every so often, they switch roles. Having a second pair of eyes acts like a continuous code review; you don’t have to schedule any separate, specific review time.

What should you look for during a code review? You might develop your own list of specific issues to check (all exception handlers are nonempty, all database calls are made within the scope of a transaction, and so on), but here’s a very minimal list to get you started:

• Can you read and understand the code?

• Are there any obvious errors?

• Will the code have any undesirable effect on other parts of the application?

• Is there any duplication of code (within this section of code itself or with other parts of the system)?

• Are there any reasonable improvements or refactorings that can improve it?

In addition, you might want to consider using code analysis tools such as Similarity Analyzer or Jester. If that sort of static analysis proves useful to you, make these tools part of your continuous build.

Review all code. Code reviews are invaluable in improving the quality of the code and keeping the error rate low. If done correctly, reviews can be practical and effective. Review code after each task, using different developers.

What It Feels Like

Code reviews happen in small chunks, continuously. It feels like an ongoing part of the project, not a big scary event.

Keeping Your Balance

• Rubber-stamp code reviews without thought are without value.

• Code reviews need to actively evaluate the design and clarity of the code, not just whether the variable names and layout are compliant to some corporate standard.

• Different developers may write the same code quite differently. Different is not necessarily worse. Don’t criticize code unless you can make it measurably better.

• Code reviews are useless unless you follow up on the recommendations quickly. You can schedule a follow-up meeting or use a system of code annotations to mark what needs to be done and track that it has been handled.

• Always close the loop on code reviewers; let everyone know what steps you took as a result of the review.