Why your team should do code reviews

Misja Alma

The problem
Your project has a nice test coverage, let’s say 85%. Your nightly build reports a wide range of metrics and they are all above your accepted levels. Your team consists of well-motivated people all willing to learn the latest frameworks and techniques. And you have adopted  Scrum, Agile, Kanban or another recent/agile software process.

But lately you have noticed  that simple new features take more and more time to implement. That despite of your test coverage, bugs have crept in your production code which can take days to solve. And that solving the highest prioritized issues is sometimes delayed because specific knowledge is located at a single person, who can only handle one task at a time.

What’s going on?
Automated testing and metrics are certainly useful, but they don’t catch everything. For instance, it is perfectly possible to have meaningless unit tests which cover 100% of your code.
You could write code with splendid metrics but which is impossible to understand by your colleagues, because method- or variable names are badly chosen.
Maybe you have written code while a colleague has already written something similar, thus missing an opportunity for reuse.
And last but not least, while there are many tools for measuring the quality of Java code, the choice of tools for non-Java code is much more limited; Think of JSF or JSP pages or new languages for which tools have not had enough time yet to develop.

Solution: Code reviews!
Code is not only written for computers, but also for humans. So why not let humans take their part in the code quality process?
Your colleague can see immediately if your code is difficult to understand or not. He can check coding standards in those areas where tools are still missing. And there’s more:

  • He could spot bugs in your code
  • You might have misinterpreted or missed a functional requirement
  • Your colleague might know of a cool third party library you could have used

On top of that, by letting colleagues read each other’s code, knowledge of the code base will be spread thoughout the team.
And by discussing suggested improvements of the reviewed code, general coding knowledge is spread through the team as well.

Tips for how to do it
In my experience, there are a few things which work well when integrating code reviews into your software development process:

First, arrange a brain storm session for your whole team, and agree on a set of review criteria.
By agreeing as a team on a set of review criteria, you prevent discussions later on.
Think of such things as:

  • descriptive variable- and method names
  • meaningful unit tests
  • conformance to gui standards
  • non functional requirements such as performance or security

When assigning reviews, make sure they are assigned to a random colleague. After all you have agreed on a set of standards to review on, so a junior colleague might just as well review your code as a senior. And, different people have different points of view, so it’s good to have many different colleagues review your code.

Look for integration with your bug tracking tool. Code reviews become much easier if commits can be traced back from a bug report or feature request.
For instance, Jira has a nice integration with Crucible which enables you to assign reviews to teammembers straight from a Jira issue.

Alternatives
The major disadvantage of code reviews is that they take extra time. It is my personal opinion that this investment in time will pay itself back with interest.
However, if you find it impossible to convince your manager or colleagues to allocate the extra time, then doing selective reviews might be a good alternative; for instance, you could subscribe to each other's commit e-mails and check those.
But, if time is not an issue, an alternative that goes a even bit further might be pair programming.

Comments (4)

  1. Ben Linders - Reply

    May 9, 2012 at 7:51 pm

    It's good to read that code reviews are still valuable, and as described fit well into agile, scrum or kanban based software development. By some, pair programming is considered to be a very intense way of code review, and when applied wisely also a very effective way.

    If you need to convince your manager that code reviews are worth the time invested, there is evidence that I collected on doing reviews. See http://www.benlinders.com/wp-content/uploads/Business-Benefits-Reviews-Ben-Linders.pdf for a 1 pager on the business benefits of code reviews.

  2. Frank Vega - Reply

    May 9, 2012 at 7:56 pm

    Misja,

    I agree with much of your reasons for doing code reviews and your suggestions for how to do them. As I read your post, I kept eagerly looking for the suggestion to "pair program" and would add do it too in a way that encourages "promiscuous pairing" as ways to accomplish your goal to addresss concerns and benefits you listed. Even when pairing on teams I've worked on we still found benefit/need/value for doing broader team reviews on occassion as well. Good stuff!

    Take care,
    Frank

  3. Stefan Lecho - Reply

    May 11, 2012 at 3:53 pm

    It seems that for Git-based projects, Gerrit is a useful tool for code reviews: http://code.google.com/p/gerrit/. Do you have any concrete experience with Gerrit ?

    • Misja Alma - Reply

      May 11, 2012 at 4:52 pm

      No I don't have any experience with Gerrit, but I have heard good stories about it as well.

Add a Comment