Code reviews provide more benefits than just bug discovery:
- code maintainability is vastly improved
- they facilitate knowledge sharing
If you write code for the long haul, have code reviews as part of your core development process.
As software developers, we strive every day to write code that is free from bugs. One thing that obviously helps avoid bugs is to do code reviews: whenever someone has authored a piece of code, someone else reviews it before it goes into the code base. Yet, few teams have code reviews as a core part of their development process. Why is that?
Why would you omit code reviews?
For some teams, it may not be a conscious choice not to have code reviews. Most schools do not include code reviews in the development process they teach, and since few teams do them, you can easily spend quite a few years in the industry without encountering code reviews. Thus, teams may omit code reviews solely because it does not occur to anybody on the team that reviews might be a thing, let alone that they might be worth the effort.
Other teams consciously decide to omit code reviews because they do not consider them worth the effort.
Code reviews certainly require effort, and for them to be effective, they require substantial effort (a team that tries to adopt code reviews as part of the development process realises this very quickly). So, the benefits that such reviews provide must be equally substantial, or they will be a waste of time.
How do we measure these benefits? In my experience, there are two popular approaches to doing this: if a team is very serious about improving their development process, they will try out code reviews for a fixed time period. At the end, they will count the bugs found in reviews and equate that to the benefit of the reviews. Teams less hell-bent on gathering empirical data may count all the bugs that have been identified in their software recently (in production, through testing, or otherwise) and consider the identification of these bugs an upper bound on the benefit of code reviews, since code reviews might have identified these bugs earlier.
Now, if a team is building software for space ships, then finding bugs is very valuable and they are likely to conclude that code reviews are a great tool. However, most teams write applications that are slightly more mundane, where occasional bugs are tolerated. Thus, the latter teams are likely to conclude that code reviews are too costly.
Code reviews are about more than bugs
Doing code reviews mainly to remove bugs is a fallacy. Research shows that only 15% of review comments pertain to possible bugs in the code. The rest of the review comments point out opportunities for improving clarity and readability to ensure long-term maintainability.
The value of improved readability alone is enormous, since reading and understanding the existing code is a prerequisite for making any sort of changes, be they bug fixes or new features. Long-term maintainability is vital as well because it helps avoid costly rewrites.
Finally, code reviews are a great vehicle for spreading institutional knowledge and sharing tips and tricks. I have on multiple occasions been amazed at how quickly I could add junior developers to very experienced teams and make them productive team members. I attribute a great deal of this success to an efficient code review process.
Conclusion
Code reviews are not only about bugs. When evaluating whether code reviews should be part of your development process, remember that code reviews have other benefits than just identifying bugs. If your team writes code that needs to be reasonably bug-free and maintainable for years, then code reviews should probably be a part of your development process. This is even more true if you occasionally add new team members.
When doing code reviews I find myself referring to a lot of “authoritative” sources for
information on design patterns, principles, practices, code smells, etc. To avoid having
to paraphrase these sources in my code reviews, I provide links straight to the source itself.
Looking up these sources takes time and requires a mental context switch during the code review.
To alleviate this I use a searchable collection of links: http://review.maintainablecode.com.
Feel free to send me a link if you would like to have something added to the collection.
Code reviews have been around at least since the late ’70s. These days, most software development teams worth their salt do code reviews. However, not all code reviews were created equal, and one particularly prevalent type is worthless at best.
The type of code review I have seen the most is what I like to call the “interactive” code review. Here, the author and the reviewer sit down together to go through the code. If they are co-located, the review will often take place at the author’s desk. Otherwise, it might be facilitated by video conferencing software. While this is typically a speedy process, the interactive code review process has a number of drawbacks, that I would like to highlight:
The author tends to take the driver’s seat
When the reviwer and the author sit together, the author tends to take the driver’s seat, guiding the reviewer through the code. This often happens because the author is eager to show off his work or because he has been toiling with this task for a while and is aching to close it and move on.
While the author gives his whirlwind presentation, the reviewer has to
- understand the problem that the author is addressing.
- think critically about the solution.
- wonder if the code would actually be understandable without a guided tour.
Having to juggle all these tasks is a tall order, and the reviewer will typically end up making assumptions and easing off on the critical thinking. This, of course, greatly reduces the quality of the review. In addition, a guided tour of the code is counterproductive when the reviewer is supposed to asses whether the code is easily readable and understandable.
Social pressure
Research carried out by Microsoft Research shows that the reviewer’s position in the social hierarchy of the team influences the quality of the code review, causing a reviewer to be less critical of code written by someone higher in the social hierarchy. This effect is likely to be exacerbated when the author and the reviewer sit together.
Lack of flexibility
To do a review together, the author and the reviewer have to agree on a free time slot for the review. This leads to interruptions and a preference for short code reviews. It also means that having multiple review iterations and having multiple reviewers becomes a real pain.
Introspection
It is not all bad news, however. As the author goes through the code and explains it to the reviewer, he revisits and articulates his own thought process. This may lead him to realise an error in his reasoning or a fault in his assumptions, or it may just highlight that some part of his code is not as easily understandable as he initially thought. Such experiences teaches the author about himself and his habits. As Galileo said: “You cannot teach a man anything; you can only help him to find it within himself.”
Despite this, having to interrupt a colleague for a code review seems like a high price to pay to have the author go over his own thought process. After all, he might as well have have been talking to a rubber duck or a cardboard cutout dog.
Code reviews done right
So, how do we do code reviews more efficiently? Well, you can ask yourself: what would you do, if the matter to review was a quote for a client, or a piece of prose, and you were the author? I bet you would let the reviewer read the prose in silence and in his own time. After all, what you are after is his experience reading it, his assesment of your work. Is it a pleasant read? Is it easily understandable? Is it correct?
This leads us to the conclusion that efficient code reviews should
- be left solely to the reviewer.
- be done asynchronously.
Allowing the reviewer to do the review in his own time means that he can pick up the work when it suits him, and he can take the time he needs for a thorough review. It also means that having multiple reviewers becomes much more feasible.
The experience of a reviewer doing a review in isolation is comparable to that of a developer, who later has to pick up the code to make changes. Thus, this is a good test for whether the code is actually maintainable. Moreover, since the reviewer will have to communicate with the author in writing, any questions raised or decisions reached during the review will be documented for future reference.
Most version control services provide some mechanism for reviewing code in this manner. Bitbucket and Github both provide excellent tools for reviewing pull requests in this manner. Even if you are not using one of these services, you still have lots of options.
Conclusion
Avoid the interactive code reviews. They are, at best, a waste of time. To be effective, code reviews must be carried out on the reviewer’s terms.
The software development industry, being a part of the general tech industry, is obsessed with new technological advances that promise to make our work simpler and us more productive. Every day, new tools, frameworks and libraries are released, marketed, hyped, and blogged about. Most of them promise to increase productivity and to reduce complexity. Most deliver as promised in the short run, but none will save us in the long run.
With the current shortage of programmers in the Western World, pretty much anyone can get a job banging out a couple of thousand lines of incoherent, kafkaesque gibberish, and then move on to the next task. If they keep up with the new, shiny tools in their particular area of expertise, they may even come to be considered good at what they do. And since the demand for programmers is unlikely to be satisfied anytime soon, they can even make a living from this for a while. I should know - I did it for years.
Maintainable code
However, if you have made a mess, it will eventually catch up with you. No tool, library, framework, or language will save you from the burden of complexity in the long run. And, after all, in the long run, it is the long run that is really interesting.
Hence, this blog is about not making a mess. It is about surviving in the long run, co-existing with, maintaining, and evolving the code you once wrote. Succeeding, more than anything, requires discipline, prudence and humility. It requires putting in an effort to write clear, concise, readable code. All the time. Every day. This is no easy feat.
Hence, Maintainable Code.