Code reviews have become an industry standard and are part of the development process at companies ranging from small startups to large corporations like Google, Facebook or Microsoft. According to a 2019 Stack Overflow survey approximately 76,4% developers review code at work. While they must provide some obvious benefits to have achieved such a broad adoption, not every developer is a fan. Some consider them a roadblock and hate them with a passion.
In this post I want to discuss the real benefits of code reviews and find out why they don't seem to work in every team and what you can do about it. To dig into this topic, let's start with a little history on how code reviews have become a thing.
History Of Code Reviews
The first ideas for code review processes emerged in the year of the Apple I release and the introduction of 5.25 inch floppy disks. In 1976 Michael E. Fagan published the article "Design and code inspections to reduce errors in program development" in the IBM Systems Journal, Vol. 15. It describes a process for preventing defects in the final product by reviewing the design of the software as well as the actual code. The code inspection is performed by a team consisting of 4 members with different roles (a moderator, a designer, a coder and a tester). They review each and every instruction at an estimated speed of 150 lines per hour and a maximum session duration of 2 hours. The final output of this process is a written report like the one below.
Sample code inspection report from the article "Design and code inspections to reduce errors in program development" by Michael E. Fagan, published in the IBM Systems Journal, vol. 15.
There is not much evidence that code inspections received a broad adoption in the industry. The most obvious reason would be that formal code reviews were too time consuming. In the report shown, it took 27.3 person-hours to review 348 lines of code, which would be insane by today's standard. We have to keep in mind though that the tools and programming languages used nowadays differ significantly from the ones in 1976. For example, the check list provided in the article contains many items that would be caught by modern compilers/linters (e.g., "1. Are All Constants Defined?") or are no longer relevant (e.g., "8. Are Registers Being Restored on Exits?").
Modern software based code reviews, also known as informal or peer code reviews, work very differently. Instead of choosing a point in time when the entire code base is reviewed, changes are reviewed before they become part of the code base. Both parties do not sit together in one room anymore, but work asynchronously and leave comments for each other in a review system. The first tools to support such a software based review appeared around 2003 as desktop applications or plug-ins for IDEs and were later largely replaced by website-based solutions. The basic principle hasn't changed much since then.
Advantages of Code Reviews
When talking about the benefits of code reviews many developers think about reducing the number of potential bugs that reach the production code. This was also the original motivation for the invention of inspections in 1976:
The inspection is not intended to redesign, evaluate alternate design solutions, or to find solutions to errors; it is intended just to find errors!
Michael E. Fagan
Scientific research in this area has shown that the motivation and benefits of modern code reviews are different today. The study Code Reviewing in the Trenches: Understanding Challenges, Best Practices and Tool Needs from 2016 analyzed the code review process at Microsoft and discovered that finding defects is only the second most important motivation. A similar Microsoft study Expectations, Outcomes, and Challenges
Of Modern Code Review 3 years prior also found other benefits in addition to identifying defects. This effect is not limited to Microsoft, Google also did some research and published Modern Code Review: A Case Study at Google in 2018 describing similar results at Google.
Let's take a closer look at some of the ways you can benefit from code reviews.
Code Improvements / Better Solutions
As a software developer, you must constantly educate yourself to keep up with newly developed frameworks, tools, or even languages. The way software was developed ten years ago is very different from today. As a result, an individual developer can never know everything and may not always be aware that there are better solutions to a problem they are currently solving. Code reviews are a great way to point out small simplifications or improvements that will greatly improve the maintainability of your code.
Finding Defects
The main reason code reviews were invented was to find bugs. Nowadays, the importance of this motivation can vary greatly depending on the project you are working on. If your team is writing software for an embedded device that can't be updated remotely, finding potential bugs is much more important than for a website that can be deployed in seconds. In recent years, this aspect has become less important, as more and more errors can also be found automatically. In addition, there is a growing tendency to focus on the conceptual structure of the changes, and not necessarily on every little detail of the implementation. Regardless of the priority this aspect has in your team, it is certainly better to find bugs at this stage than to have a customer complain about them.
Knowledge Transfer
This is perhaps one of the most important aspects of code reviews, especially in small teams. As part of the process, the reviewer needs to understand the code written by someone else, figure out the motivation behind it and perhaps even start a discussion to understand further details. This transfers knowledge from the code author to someone else on your team and prevents the author from becoming a single point of failure. If a developer is unavailable (vacation, illness, ...) and a bug in one of their components needs to be fixed urgently, there should now be at least one other knowledgeable person for that area. This greatly reduces the risk of adding more defects when making changes to existing code.
Maintaining Norms
Every code base has its own set of norms, whether in the form of style guides or implicit rules about how certain problems should be solved. Some of them can and should be enforced by automated tools, but many of them are too complex and require a human review. For example, when you are working on a project for a long time you will notice that code consistency is one of the key points to achieve maintainability. While it is usually impossible for software to compare the intention of two code fragments, an experienced developer can often easily tell during a code review that you're reinventing the wheel.
Team Awareness / Share Ownership
Working on a mission critical component can be stressful, especially when real world testing is difficult. When you do code reviews you know that at least one other person is looking at your changes and may catch bugs that you didn't notice. It is also a great way to interpret code as a result of teamwork and to be less protective of it.
Cartoon "Good Questions" by Geek & Poke, provided under the CC-BY-3.0 license.
Misconceptions about Code Reviews
We have discussed several benefits of code reviews and how they can help improve your development process: Your software has fewer bugs, is easier to extend, and all developers are more familiar with the code - thus making it less problematic when developers in key positions leave a team. If there are so many benefits to performing code reviews, you may wonder why some developers hate them.
In my experience, negative opinions about code reviews usually arise when developers try to use code reviews to solve problems that would be better addressed in other ways. If you expect your reviewer to verify tiny details in your code that could be checked by automated tools, your team will have an unpleasant experience and dislike code reviews. The focus of code reviews should be on the logic and maintainability of the code.
Here is a list of some things that should be handled outside of the review process or that you should be aware of.
Preventing Build Breakage / Test Failures
A reviewer should not have to stop their work only to find out that the code does not compile or that the tests fail. You should either set up a CI that builds the software and runs the tests or use pre-commit hooks to prevent broken code from being committed. Maintaining a comprehensive set of unit and integration tests can also catch bugs early and provide some confidence that the code meets the design requirements without forcing the reviewer to manually review each case.
Styling/Formatting
There are many different code styles and some developers tend to have strong opinions about them. You should try to decide on one and then use tools to format the code / enforce the style. This can greatly reduce the number of revisions as each change may introduce a new style violation. Nitpicking about style issues can also be used as an excuse not to review the actual logic. If using automated tools is not an option, try instead to add a rule that you focus on other aspects of the code before commenting on style issues.
Linting
Every time a reviewer finds an issue in the source code, two team members are interrupted in their work. The author of the change must pause their current work, read the comment, checkout the corresponding branch, fix the issue and push the changes. Then the reviewer has to open the new revision and verify the fix. These interruptions can have a significant impact on your team's efficiency. Tools that can catch certain classes of bugs (such as linters) before the reviewer starts analyzing the changes are a great way to reduce the workload. If you encounter recurring issue patterns in code reviews, you may want to investigate whether you can configure or extend existing linters to detect them automatically.
Not All Team Members Participate In The Process
To get the most out of your code review process, everyone on the team should participate. Skipping code reviews for the team leader or limiting them to junior developers prevents knowledge sharing and sets the wrong mindset. New team members can come up with fresh ideas during a code review and senior developers will still introduce bugs from time to time.
Challenges
Code reviews can greatly improve your processes and teamwork, but that doesn't mean they're without their challenges. I think the two most common ones are:
Code Reviews Are Time Consuming
Reviewing code will always take some time and cannot be fully automated. Tools may find logical errors, but they cannot, for example, check whether an implementation meets the requirements. If reviews tend to take forever, this is usually a symptom of other problems. Possible causes include changes not being split into proper commits, disagreements about certain processes / coding standards or unclear requirements. While these issues become very visible in code reviews, they also have an impact on the maintainability of your code. So I would suggest digging a little deeper if your code reviews are taking too long.
Tension Between Reviewer and Author
The reviewer acts as a gatekeeper and criticizes the author's work via text messages. With this type of communication, there is always a risk that the other person will misinterpret statements and that comments will be taken more negatively than they were intended. This can easily lead to tension and hurt feelings. If the code requires major changes or the discussion gets too heated, you may want to move to a face-to-face meeting or video call. This makes it easier to find solutions and avoids the "author vs. reviewer" mentality. As a reviewer you can also try not to focus only on the negative parts...
Cartoon How To Make A Good Code Review by Geek & Poke,
provided under the CC-BY-3.0 license.
Conclusion
Code reviews are a great tool for improving code quality, sharing knowledge and fostering teamwork. It is therefore no wonder that they have become so widespread. However, whether you actually get these benefits depends on how you implement code reviews. You should try to automate all the boring aspects (run tests in a CI, validate code style via a formatter, use linters, etc.) and let developers focus on whether the code meets the requirements and is maintainable.
Code reviews also tend to uncover other issues in your development process. If you are having problems with your review workflow, you may want to dig a little deeper and find out if reviews are the real cause.
Top comments (0)