As a programmer I never had the pleasure of working on a team that had effective code reviews. In fact, I don't ever recall being in a code review. I certainly remember a lot of lip service being paid to the importance of code reviews, just never actually saw one. Maybe that was because of bad management, or maybe code reviews weren't as important as we thought, or maybe we tried and failed, or maybe we were having code reviews every day but they were so horribly unrecognizable that I just thought they were something else.

Later in my career, I still believed code reviews were important, but mostly dismissed them because I never knew how to make one work successfully. In fact, I wasn't sure what success for a code review was. I thought of code reviews as one developer presenting her work to the rest of the team, and the team in turn making comments and suggestions of which most would be difficult or impossible to implement or maybe even make management throw up a little in their mouths. I also pictured the scope of the work being reviewed to be way beyond the scope of the meeting making it impossible to get any real analysis done in 1 to 2 hours.

Well I'm happy to announce that I was wrong and code reviews can in fact be very successful and for us, success is defined by the following criteria:

• Everyone learns something
• No one feels attacked
• We reduce knowledge concentration risk
• We discuss craftsmanship
• A non superficial understanding of the code under review is achieved by all
• Suggestions are easily implemented within an iteration (for us that's 2 weeks)
• We get all this done in 1 hour

We achieve this by:

• Making the review about a metric and not a component
• Selecting a unit of code as small as a single method
• Discussing the smells associated with the code, particularly ones which affect the selected metric
• Discussing the refactorings necessary to improve the code which in turn improves the metric
• Creating dev tasks for those refactorings to be implemented in a future iteration

An example focused code review goes like this. The team selects a metric they would like to use as the lead in. It is important to remember that the metric is the starting point, not the focus. We want to remember that we're meeting to improve our code, our standards and our practices, not our numbers. The selected metric then selects the unit of code for us. Sometimes this is a method, sometimes a class depending however it's rarely bigger. We write up a list of code smells that we can detect in the code unit and pay careful consideration to those which affect the metric. We then select the appropriate refactorings to improve the code. Usually we detect smells right from the Martin Fowler text, Refactoring, other times we coin our own smells which are then documented in our wiki. Dev tasks are created for the refactorings and prioritized against our backlog. When the refactorings are actually implemented they may be done by anyone in the group. Sometimes the original author does the work, sometimes she may be part of a pair and other times she may not be involved at all. The main concern is that we expose the problem in the code, and in the practices that created that code. Although it will eventually be fixed in the code, it's more important that it's fixed in our practices first.

Some fun metrics to start with are:

• NCSS (Non Comment Lines of Code): the measure of lines of executable code in a method. The bigger the method, the higher the likelihood that it will be difficult to understand, and that it's likely doing too much.
• CCN (or CCI, Cyclomatic Complexity Index): An indicator of the number of paths through a method. A Higher number represents a higher complexity. Which means the method should be refactored into smaller methods.
• Churn (or Developer Activity): the number of times the file is checked in. A relatively high number implies that the class is responsible for too much.

It is a good idea to change the metric you're using as different metrics will inspire different conversations.