How to Perform Effective Team Code Reviews

Editorial Note: I originally wrote this post for the NDepend blog.  You can check out the original here, at their site.  While you’re there, check out all of the new tech debt-related features in the newest version of NDepend.

I’ve heard people say (paraphrased) that teams succeed uniformly, but fail each in its own unique way.  While I might argue the veracity of this statement, it evokes an interesting image.  Many roads, lined with many decisions, lead to many different sorts of failures.

Code review presents no exception.  Teams can fail at code review in myriad, unique ways.  And, on top of that, many paths to broader failure can involve poor code reviews (doubtless among other things).

How can I assign such importance to the code review?  After all, many would consider this an ancillary team activity and one with only upside.  Done poorly, code review catches no defects.  Done well, it catches some defects.  Right?

How Code Review Can Go Wrong

Simply put, code review can have worse than zero effect.  Ineffectual code review suffers mainly the opportunity cost of the participants’ time.  But toxic code review creates morale problems, counterproductive team dynamics, and damaging distractions.

So the first order of business is to avoid a net negative effect.  To do this, one simply has to remove the potential of toxic culture from the process of code review.  Of course, that’s a bit easier said than done, but a lot of it just means following basic rules of human decency.  Start by treating one another with respect.  Then ensure that all participants feel comfortable getting and receiving feedback.  Enlist the help of even-tempered, charismatic folks to lead by example.

Once you’ve insulated yourself against the most damaging effects, it’s time to guard against ineffectual code reviews.  It is toward that end that I’ll be focusing for the remainder of this post.  Ineffectual code reviews can ways time, as I mentioned earlier.  But they can also create a false sense of security and lead to poor choices.

So what makes code review effective?  How can your team get the most out of this activity?  I’ll offer some thoughts based on firsthand experience across a wide number of organizations.

Avoid Trivia and Superficial Concerns

For starters, you need to make sure you don’t fixate on trivial matters.  If you have people spending time on these things, make sure they have real significant for the participants.  Whether you have single person, asynchronous reviews as part of your process or whether you do “mob style” code reviews where everyone teams up, spend your time effectively.

What constitutes a trivial review?  Obviously this invites subjectivity.  But let’s consider some examples.

Do you find yourself commenting routinely on whether code follows casing and naming conventions?  Are you saying things like, “someone could pass in a null argument and break this method?”  How about arguments over whether code would compile or return what the author thinks it would return?

Why do I refer to these things as trivia?  It’s not to suggest that they carry no importance.  Rather, I’d submit that these things should be sorted out well before code review time.  For the source code they matter, but for live, paid discussion, these are trivial concerns.  Automate these things.

Anything that contributes to your team standard or that you could demonstrate with tools like tests, code contracts, or even the compiler is something you should automate.  Discussing these things at code review both wastes time and distracts from issues better suited for human contemplation.

Offer Uplifting Thoughts and the Opportunity to Learn

Once you’ve purged the trivial your review process, focus on the positive.  Do people learn at your code reviews?  Do they routinely leave having picked up useful, actionable information.  Perhaps you’d even go so far as to say that people enjoy them?

If not, why not?

As developers, the overwhelming majority of us got into the field for at least some love of the game.  Maybe we enjoy the feedback loop and the state of flow.  Or perhaps we crave the precision and logic of animation.  Whatever the motivation (unless it’s purely money), coming away with a better approach or a time-saving shortcut energizes us.  Shoot for this dynamic in code reviews.

Try to move away from the “code review as exam” anti-pattern.  You will get limited to negative returns out of an approach where the senior developers ‘quiz’ the newbies and ‘sign off’ on what they do.  This instills learned helplessness, threatens toxicity, and misses the larger opportunity.

Focus on good techniques and cool things that folks have done.  Use the opportunity of code review as a chance to offer praise and share approaches with the wider team.  Setup situations where everyone comes away having learned something.  Make at least part of the review about celebrating the code and not just picking it apart.

Go Conceptual and Evidence Based

Last, I’ll talk about what I think of as the meat of the code.  Up to this point, you might think that I’m some kind of new-age proponent of code reviews filled with nothing but feels.  Far from it.

In the previous sections, I talked about what to avoid in order to have successful code review.  Mainly, avoiding the toxic, the superficial, and exclusively negative focus means avoiding unnecessary conflict.  But don’t confuse my message of avoiding unnecessary conflict with the message that no conflict is necessary.  Smart, passionate human beings will disagree from time to time.  Just make sure that those disagreements are about matters of substance rather than politics or sniping.

Perhaps the main reason for me to talk about code review on the NDepend blog is how I use NDepend in code reviews.  NDepend basically serves as my textbooks in a vigorous academic debate.  I use dependency and heat diagrams to help people visualize my points about the architecture.  I grab out-of-the-box metrics to make points about code quality in places.  And I rely heavily on CQLinq to make ad hoc, statistical arguments about the nature of code.

These are the types of debates that are valuable at code review time — not debates over who has been at the company longer or whether you should use tabs or spaces.  Have earnest debates over design patterns, architectural approaches, code quality, and code structure.  Avoid the trivia.

Code Review is About Improvement and Not Perfection

I’ll offer one last closing thought, both more general and more philosophical.  I would also argue that all points I’ve made in this post can be rolled up into this one, broader consideration.

However you structure your code reviews, bear in mind that the result (if they’re done well) will be incremental improvement.  It won’t be perfection.  Seriously, it won’t ever be perfection.

If you come away having noticed and staved off a single design flaw, that’s a win.  If half of the team learns a cool new approach, that’s a win.  Or even if you have a thought-provoking debate about whether or not code duplication can be acceptable in unit tests, that’s a win.  Code review wins and proves effective anytime it drives an improvement.

So to conduct effective code reviews, always ask yourself whether or not they produce improvements.  Are they making the team better, or are they simply serving the process?