Why Code Reviews Don’t Work

I’m a strong believer in code reviews.  Having developers review and discuss code in development is one of the best ways to ensure good code quality and provides an excellent way to share domain knowledge with team members who may otherwise not get the opportunity.  It also allows experienced developers to share their knowledge and ideas with less experienced developers, an excellent way of distributing development skills across an entire team.

But review adoption doesn’t always stick.  I’ve seen the process taken up and then fizzle out on a number of teams usually for the same set of reasons.  The following are some of the reasons why a code review process can fail to take hold within a team.

Never Following Through
This is the biggest moral killer while trying to establish a code review process on a team.  What will generally happen here is that when code is put up for review, the author simply takes no notice of the comments or suggests and checks in the code without modification or without any follow up changes.  When other developers are taking the time to examine code and make positive suggestions they will quickly realise if they are simply being ignored.

And this kind of behaviour is both demoralising and infectious.

If developers see their comments being ignored they will very soon start to wonder why they even bother.  And if other developers don’t have to take the time to act of good suggestions, why should they.  Pretty soon you have one of the two following situations – people don’t even bother posting because nothing will come from it or you have review after review being raised and all of them being left uncommented and open for everyone to see.

Developers need to know they have the time to respond to feedback.  If they feel under pressure to move onto the next thing then they won’t have the desire (or ability) to actually respond to suggestions.  While not always possible, in the long term quality should take priority over speed, and if suggestions improve the code there should be an environment where reacting to these is important.

Reviews should always be classed as ‘finished’ when the feedback has been acknowledged and by tracking the number of open reviews it allows people to see what feedback still needs to be acted upon easily and allows people who are slipping to be picked up sooner rather than later.

Lack of Buy In
This is one of the hardest hurdles to overcome.  When a team simply doesn’t buy-in to the idea of code reviews, it will be difficult to get anything useful out of a review.  People won’t post reviews (often stubbornly refusing) or when they are the reviews will be superficial at best – finding very little of note and eventually killing the process through sheer apathy.

This can often be an attitude that is based on other problems in the team.  I have rarely met a programmer who didn’t want to talk about code, or who didn’t want to offer you a suggestion on how something could be improved or optimised.  The lack of desire to talk about their work, when it’s across the entire team, could be systematic of wider problems in the team.

But if the attitude of the team is generally positive and code reviews are the only sticking point, then it’s going to be less of problem trying to resolve these when trying to introduce a process into the team.

If there is a general malaise towards a process, strong leadership is one of the best ways to get other members of the team interested in a process.  Notice that I didn’t say strong management.  Leadership is a whole different ball game.

You can also start with those who are interested, keeping reviews small but public (they should rarely be private anyway).  As other members of the team see people in animated discussions on how something was implemented, it’s a unique programmer who doesn’t want to get involved at some level.

Unrealistic Expectations
I’ll go on record and say that you will rarely find bugs in code you review, especially if you are reviewing all or most of the code being written.  Or, at least you shouldn’t because your ‘bugs to code written’ ratio shouldn’t be high enough otherwise you have more serious problems.  But when a team is new to code reviews they can often have an expectation that issues will be found and they will be found often.  Which can cause a team to question what they are getting out of reviewing everything that’s being written.

But I’ll also go on record and say that code reviews can prevent bugs.

Code reviews benefit teams in the long term more than they do in the short term.  You might not be finding bugs on all your reviews but over time, if people are talking about code and sharing their experience, your team on the whole will become better programmers.  They’ll think that little bit longer before committing code if their peers are going to be looking at it, and they’ll have their bad habits smoothed out review after review.

And all this will lead to less bugs being added to the code base – but this is one of the hardest things to grasp when a team starts out and is looking for short term gain.

Peer Fear
This can manifest in a number of ways.

The first is through a fear of criticism where a developer is simply fearful of the criticism they will receive when putting their code up for review.  Unfortunately this isn’t always in the hands of the poster.  If a team has even one aggressive reviewer, this can effect the entire process, with people hesitant or even refusing to post their code, and when one reviewer starts to control the process, the ability to share knowledge decreases drastically.

The other side of the coin is where developers are worried about disagreeing with each other.  Comments on reviews are not always correct and not always the best course of action.  But if the team has developed a routine of automatically acting on suggestions (either from a subset of developers or all of them) then in effect any productive discussion is squashed and if people are afraid to say no, the code base isn’t always going to go in a good direction.

When this happens, the process can quickly lose steam.  If reviews cannot start a worthwhile discussion then the worth of reviews will decrease drastically.  If people see the code base is not improving (or even worse, degrading) they will not continue to post up reviews and the process will simply die out.

This is usually a management issue.  Domineering peers will destroy any process, but this can usually be resolved by taking that peer to one side, discussing the reasons behind the process and explaining how they can still be part of it while not taking over the discussion.  For those times where one developer is lacking the confidence to disagree, then it will often fall to other developers or the team lead to do the disagreeing for them, at least from the start.  As they start to see that disagreeing isn’t a negative trait in a review, they will start to feel more comfortable in reviews and discussions will start to flow.

Solutions?
Everyone likes a quick fix solution, but as with anything worth doing, it’s not always easy to solve these problems, especially one as difficult as a lack of buy in into a new process.  But I’ve generally found that most problems, like many team issues, are beaten by strong leadership and by being open about why the process is being introduced and how, in the long term, the developers lives will improve as a direct result of being part of it.

But to be honest, if you’re trying to solve these problems and you don’t buy into it yourself, then you’re onto a losing streak either way.  If you don’t believe in the process, then chances are you’re not to worried if it falls by the way side.  Which is fair enough.  Code reviews are not a mandatory part of software development and if the literature on the benefits of code reviews doesn’t persuade you, then nothing will.

Though I’d recommend you read them all again.