Code Reviews

Ever since I’ve read the book “Best Kept Secrets of Peer Code Review” a couple of years ago, I was cottoned onto the idea that code reviews are good things to have. The book cites many successful examples, like Cisco and other successful corporations, and surely they must be doing something right that is worth emulating?

When I first mentioned about my plan for a code review, one of the first things I wanted to know from the team was to find out their own experiences with the process. Among the various discussions we’ve had, one recurring topic that keeps getting cited as an issue for a code review is the lack of domain knowledge, or the ‘Bikeshed Problem’, as one of my colleagues puts it.

And he tells it like this: if you have a large development team, it’s likely that the person assigned to review your code may not have sufficient domain knowledge to critique on the changes you’ve made. However, when you’re told that your job is to ‘find fault’ with others, chances are, you will find them.

The lack of domain knowledge can hinder the process, not help it. It is plausible to believe that the only problems that you’ll be able to spot are only the easiest ones - things like formatting, code convention and spelling. At the end of the process, you’re most likely made not much improvement to the code, irritated your fellow colleague with trivialities, and wasted your own time.

He has a valid point.

Code reviews are meant as a process for locating defects, not a process of bickering over changes and causing friction among fellow developers. And even with the best of intentions, it is sometimes inevitable that frictions can occur within a code review process, given a code review is a task that relies on one’s communication skill, and not solely a technical one.

But if a code review process can turn terribly wrong, does that mean it is be better than having no code review at all?

I don’t believe so. Since we know that there bad ways of conducting a code review, we will assiduously avoid them. If we find something that works for us, we keep them. And we’ll continually refine this process until it becomes a natural, unawkward and effective routine.

So far, even after months, this process remains as a ‘work-in-progress’, although there are a few emergent behaviours that are becoming clearer:

1. A code review process should not be all about spelling, formatting and comments about non-adherence to convention. if one is irked by spelling and formatting, then the reviewer should take the liberty of fixing it himself; they are not proper critique for a code review;

2. Code reviews are assigned to people in a random fashion, irrespective of how familiar the reviewer is with the domain of the code. This is to mitigate what we call a ‘hit by a bus’ problem, where if a developer is the sole person who understands the code gets hit by the bus, then it becomes difficult for someone else to take over the code. (Not to mention that we’ll be very sad about losing a colleague too.) Besides the purpose of risk mitigation, the reviewer gets an opportunity to ask questions and further his own understanding of a different subsystem of the software;

3. The ‘eject’ button. Sometimes, the code can be too far off for a reviewer to understand without going deep into understand the design, architecture and nuances of the problem. If it is time consuming or difficult to review the code effectively, the code will be reassigned to somebody else who has better a domain knowledge understanding. And it works in our situation because our team works loosely in pairs, and at times even perform pair programming when dealing with larger and more intractable problems.

But I have no doubt about the merits of a code review process, although we should remember that part of the impact of a code review is a psychological one: people learn to code better through this process because of the behaviourial impetuses that drives them to work better. Because we’re talking about people’s pride, fear and ego here, there is absolutely a need to temper the process to factor any sensitivities involved.

The most important lesson I’ve learnt about a code review is that whenever it is a process that involves individuals, you’ll always have to find your own right balance in conducting the whole process. Find out what works for you by basing your decisions on the nature of the software project and the team that you have - so don’t be foolhardy to believe that all code reviews are the same, and there can be a ‘one size fits all’ solution.