Code Review: More Than Just Finding Bugs

Code reviews have become an industry standard and are part of the development process of companies ranging from small startups to big corporations like Google, Facebook or Microsoft. According to a Stack Overflow survey from 2019, about 76,4% developers review code at work. In the first entry of our series on code reviews, we want to find out why they are so prevalent and what to watch out for when you introduce them into your workflow. If you think the main focus of code reviews is to find bugs you should definitely keep reading.

History Of Code Reviews

The first ideas for code review processes surfaced 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 how to prevent defects in the final product by reviewing the design of the software as well as the actual code. The code inspection is carried out 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 with 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 following.

There is not a lot of 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 shown report it took 27.3 person hours to review 348 lines of code, which would be insane by todays standard. We have to keep in mind though that the tools and programming languages used nowadays differ significantly from the ones in 1976. The check list provided in the article, for example, contains many entries that would be detected by modern compilers / linters (e.g., “1. Are All Constants Defined?”) or are not relevant any more (e.g., “8. Are Registers Being Restored on Exits?”).

Modern software based code reviews, also called informal or peer code reviews, work completely different. Instead of analyzing a specific source code state, only the changes between two versions are displayed to the reviewer. 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 working principle didn’t change much since then.

Advantages of Code Reviews

When talking about the advantages of code reviews many developers think about reducing the number of possible bugs that reach the production code. This was also the original motivation why code inspections were invented 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 found many different advantages as well. This effect is not only limited to Microsoft, Google also did some research and published Modern Code Review: A Case Study at Google in 2018 describing similar motivations at Google.

Let’s take a closer look at some of the ways in which you can benefit from code reviews.

Code Improvements / Better Solutions. As a software developer, you need to 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’s. A single developer can therefore never be all knowing 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, which will greatly enhance the maintainability over time.

Finding Defects. The main reasons why 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, it is much more important compared to a website that can be deployed in seconds. In recent years, this aspect has lost some of its importance, as more and more errors can also be found automatically. In addition, there is a growing tendency to check primarily the conceptual structure of the changes, but not necessarily every little detail of the implementation. Regardless of what priority this aspect has in your team, it is certainly better to find errors 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 maybe even start a discussion to understand further details. This transfers knowledge from the code author to another person in 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 another 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 norms, this can be in the form of style guides or implicit rules 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 need to be verified by a human. 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 tell quite easily during a code review that you’re reinventing the wheel.

Team Awareness / Share Ownership. Working on a mission critical component can be quite stressful, especially when it is hard to do real world tests. When doing code reviews you know that at least another person is looking at your changes and may spot any bugs that you didn’t notice. It is also a great way to interpret code as a result of team work and be less protective about it.

Misconceptions about Code Reviews

We have discussed various advantages of code reviews and how they can help improving your development process: Your software has fewer bugs, is easier to extend, and all developers are familiar with the code - thus making it less problematic when developers in key positions leave the company. If performing code reviews offers so many benefits, you may wonder why there are still critics.

Based on our experience, negative opinions regarding code reviews usually rise when developers try to solve problems using code reviews that would be better addressed in other ways. If you expect your reviewer to verify tiny details in your code which could also be checked by automated tools, your team will not have a pleasant experience and dislike code reviews. The main focus of code reviews should lie on the logic and maintainability of the code.

If you start discussing the position of every comma, the chances are high that code reviews drag on forever. This leads to stress as the progress comes to a halt and maybe even results in tension between different team members. Keep in mind that every time the reviewer or author of the code needs to interact with the review system they need to interrupt their current work. The aim should therefore always be to minimize the number of revisions required before merging a change into the production code.

Here is a list of a few things that should be handled outside of the review process or that you should pay attention to.

Preventing Build Breakage / Test Failures. A reviewer should not interrupt their work just to find out that the code does not compile or that the tests are failing. You should either setup a CI that builds the software and executes 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 requiring the reviewer to manually review each case.

Styling / Formatting. There are a lot of 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 can introduce a new violation of the style guide. Nitpicking on style issues can also be used as an excuse to not review the actual logic. If using automated tools is not an option, you should instead try to add a rule that you first focus on other aspects of the code before commenting on styling issues.

Linting. Every time a reviewer detects an issue in the source code, two team members are interrupted in their work. The author of the change needs to pause their current work, read the comment, checkout the corresponding branch, fix the issue and push the changes. Afterwards the reviewer needs to open the new revision and verify the fix. These interruptions can have a significant impact on the efficiency of your team. Tools that can detect certain classes of bugs (e.g., linters) before the reviewer starts analyzing the changes are therefore a great way to reduce the workload. If you encounter recurring issue patterns in code reviews, you might want to investigate if you can configure or extend existing linters to detect them automatically.

Not All Team Members Participate In The Process. If you want to get the most out of your code review process, everyone in 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 more senior developers will still introduce bugs from time to time in their code.

Challenges

Code reviews can greatly improve the processes of a development team and enhance team work, but there are also a few challenges that you may face. Describing them in detail here would exceed the scope of this post.

We instead plan follow-up articles that go into great detail on how to best perform code reviews. They will contain practical tips on how to handle possible challenges. As a small preview, we list two of the most common problems here.

Code Reviews Are Time Consuming. Reviewing code will always take a bit of time and can not be fully automated. Tools might find logical errors, but they can not check, for example, whether an implementation fulfills the requirements. A certain amount of time is required for a thorough review, but if the process tends to take forever then this is usually a symptom for other issues. Possible root causes are that changes are not split into proper commits, disagreements about certain processes / coding standards or unclear requirements.

Tension Between Reviewer and Author. The reviewer acts as gatekeeper and criticizes the work of the author through a text message based tool. 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 actually meant. This can easily lead to tension and hurt feelings. A first step to improve this situation can be to not only focus on the negative parts…

What’s next?

We have now established why you should do code reviews in your team, but only gave a rough idea on how you should do them in practice. As mentioned earlier, we will change that in the next entries of our series on code reviews, and go into great detail about how to create good merge requests and how to become a good reviewer. We move to a very practical level and describe, for example, how you can perform certain code review related tasks faster with git.

If you found this blog entry informative and want to stay tuned, subscribe to our RSS feed or follow us on social media. You might also be interested in our product MergeBoard which uses a more intelligent approach to display code changes, reducing the time required for code reviews. Regardless of which tools you use, we are also here to help you improve your development processes - contact us to schedule a free consulting appointment to find out what we can do for you.