Code Review Process

If I were to answer the question, “What are the elements of a worthwhile code review?”, to a IT reporter in need, I would answer something like this:

A proper code review is a valuable tool to any software team. The longer that bugs go undiscovered, the more costly it is to fix them. For example, if a developer works out a design on a whiteboard, updates and improvements can be made very cheaply, just by erasing part of the software diagram and starting again. After the product has shipped, an undiscovered bug becomes very expensive to fix, usually through a laborious patching process. A code review is a way to reduce costs by eliminating bugs early. Code reviews occur early enough to catch bugs before they have an opportunity to become very expensive to fix.

But, in order for the code review to be useful, it should be conducted in a responsible manner by a mature team with a good leader. All developers have certainly experienced code review horror stories or at least ones that qualify as wastes of time: Reviewers come to the meeting unprepared and begin reading the code line-by-line, or, reviewers focus on silly issues like “oh, this ‘for’ statement should have a space after the opening parenthesis.”

There are ways to eliminate these sorts of time-waster. There are even ways to measure if your code reviews are worth the time spent. After all, if you’re spending more time in your code review than you could have been fixing bugs, then your code reviews are not useful. Of course, this is rare.

First of all, to eliminate the silly code review comments like spaces or capitalization, you need a good style guide. A style guide describes how the team prefers to comment, how the team prefers to name classes, variables, and methods, and how to format. It should not attempt to convey software best practices, like how best to deal with memory that can’t be allocated, or tracking threads, and so on. It should only focus on style. The style guide should be short enough so that it can be easily grasped by everyone on the team and not cover every nauseating detail so that team members still have a sense of freedom to write the code the way they see fit. After all, writing code can be very personal.

Developers are then held accountable that they will have no style issues in their code. If the team has established good style guidelines which are readily accessible and used frequently, this ought to be easy.

To ensure that everyone prepares in advance, you can create some fun rules. For example, we have simple rule that says, “If you have no red marks on your code print-out’s you’re not allowed into the review. Red marks are the ticket!”

To help the team focus on important issues, a code review checklist is useful. The team can create this together, again keeping it very short so that it’s easy to refer to, and then use it to coordinate focusing on specific areas. Too often, poorly run code reviews has everyone focus on the same superficial issues. A code review checklist can help encourage a smaller group to focus deeply on a specific area, another group to focus on a different area, and so on. This helps the code review achieve depth.

It’s also a good idea to have someone other than the author to run the code review meeting. Software developers tend to be a proud lot and an independent moderator can keep the discussion from getting too personal. A good moderator is someone good at running meetings, keeping people on track and focused on important issues.

Finally, you can calculate just how effective your code reviews are by having two pieces of information: The average bug fix rate for a team member (usually ranges between 1 and 2 per day), and how many major issues you uncovered in your code review. Assume you found five major issues in your last code review and your team’s major bug fix rate is one per day. If there was no code review, it would have cost approximately five days to fix those bugs (the number of bugs times how long it takes to fix a major issue). If your code review had five participants, each prepared for one hour, and the review meeting lasted one hour, the total time is 10 hours. That’s well less than five days, so your code review was worthwhile!

Our team in China is just now adopting these changes and we’re excited about it. We especially enjoy being able to measure the effectiveness our our reviews.

Comments are closed.