In my programming practice, peer code review has become such second nature to me that it’s sometimes a challenge to articulate why I think it is important. I was recently asked this question, which forced me to think about it, and so now I want to put it into writing, lest I forget my answers.
It’s just to reduce bugs, right?
Before I jump into the list proper, let me address what I believe is a common misconception about code review. I believe this because I, myself, once held this misconception.
That misconception is that code review is only, or primarily, to catch bugs. This sentiment may be expressed in various ways from simply expecting that two sets of eyes are more likely to detect an unnoticed mistake, to perhaps a more formalized requirement that senior team members review code of the “less trusted” junior team members, to catch their mistakes.
And to be fair, the “two sets of eyes” reasoning is what initially me to start doing code reviews on my team at the time some 10+ years ago. And it probably is a good enough reason to start with code reviews on your team, if you aren’t already. But the truth is, this is one of the least interesting benefits of code review I have experienced in my career.
And one last piece of business before the list. “Code review” may mean different things to different people. Some consider Pair Programming as a form of, or substitute for, code review. (It is neither, but more on that another time.) Some think of quarterly performance appraisals where a panel of seniors or managers tears apart your code on a large screen.
What I mean by code review can probably be best summarized with a simple rule:
- All code must be reviewed, and approved, by a team member, before being merged into the main code branch.
The key is that after you have written a new piece of functionality, or fixed a bug, a team member must read every changed line, and approve of the change.
This is easily accomplished with Pull/Merge Reuqest approval rules on GitHub or GitLab. With older tools, the same policy can be implemented verbally or by email, as needed.
And finally, on small teams, or in the case of an emergency fix, an optional second rule may be applied, to avoid a large backlog of unmerged code during an outage, or while someone is on vacation:
- In case of blocking work, code may be merged prior to review/approval, but must be reviewed by a team member at the earliest convenience.
I’ll talk about what I look for during code reviews in another post.
Let’s move on to the reasons!
In my opinion, the biggest, longest-lasting value from code reviews comes in the form of mentoring opportunities. While it’s hard to quantify, I have probably learned as much about the art of coding from peers reviewing my code, and suggesting alternatives or better practices, than I have from reading books or blog posts or Stack Overflow.
No formal mentoring program or training is necessary. Simply commenting on areas for improvement, whether it be stylistic changes, to a better algorithm, to a better way to organize code, fosters an environment of mutual mentoring. And this doesn’t just benefit the more junior developers on the team. Everyone on the team should be able to contribute.
2. Higher Quality Code
High-quality code should, broadly speaking, adhere to SOLID principles and other relevant best practices, it should honor the idioms of the language or framework being used, it should be well formatted and free of typographical errors and spelling mistakes in variable and function names as well as in comments, be easy to read and understand, etc.
This is one area where the more junior members of a team are often better at reviewing code. More than once, I have found myself writing code which I felt was well-written, only to later have another developer with less experience with the language or the problem space tell me they didn’t understand what my code did.
Ideally, the least experienced person on your team should be able to read the code written by the most experienced person on your team. If they can’t, the WTFs/minute value goes up significantly, and the more senior developer should consider re-writing their code in a more readable way.
3. Reduced Bus Factor
The Bus Factor is a popular, semi-humorous, measure of risk in a project. A piece of code or technology has a bus factor of one if there is a single person who, if hit by a bus or otherwise removed from the picture (even temporarily, as due to holidays or illness), would cause a serious problem for the project.
The practice of requiring that all code is reviewed by at least one other team member helps to increase the bus factor naturally, to at least two. Suppose Bob has been working on the new accounting system, but Alice has been reviewing all of his code. When Bob goes on holiday, or is hit by a bus, Alice will at least have a basic understanding of the accounting system, due to having reviewed Bob’s code, so that the project can continue in Bob’s absence.
For especially critical parts of a system, a team may decide that the number of reviewers should be greater than one, to further increase the bus factor. A related strategy would be to have a specific, designated reviewer (or reviewers), review all code related to a particular system, to ensure that the right people know what’s going on in the system, and will be prepared to support it in an emergency.
4. Better Communication
Another long-term benefit of code reviews, is that it helps each developer on the team to improve their ability to communication, especially about complex, abstract concepts, such as code. Ultimately, a computer programmer has one job: Communication. They must communicate their ideas with a computer, but also with other humans. By participating in regular code review, both as the reviewer and as the recipient of review, a developer hones the human aspect of this technical communication skill.
5. Fewer Bugs
And finally, coming full circle, code review can help reduce bugs.
There are other techniques which usually do a better job of directly attacking bug count, such as writing good unit tests, or Test-Driven Development, and it is rare that I review TDD-written code and also find a subtle bug with my own eyes, the cumulative effect of all of the other benefits of code review will add up to an overall reduction in defects.
Code that is easier to read and understand, is easier to extend, and will therefore have fewer bugs.
Code that is broadly understood by more developers will tend to have fewer bugs.
And better communication about code and its desired behavior will lead to fewer bugs.
And of course, there are times, as rare as they may be, where reviewing code does actually find an actual bug, often in the form of a corner case the original coder simply didn’t consider.
No doubt there are other reasons to review code, but these are the big five in my estimation. Many other reasons can probably fall under one of these categories, but even these categories don’t have clear lines between them.
What do you think I missed? What other benefits (or even drawbacks) have you experienced from code review?