What are Code Reviews and Why Should We Spend Time on Them?

A Parable

John spent days writing a software component. He tested and double-checked his code, and he was satisfied that it worked properly, according to the requirements he was given, so he checked it into source control. A few weeks later, a new version of the software that included his code was released to production. A user discovered a bug caused by John's changes. The user tweeted about the bug, and this was retweeted thousands of times. Before long, word got back to John. An edge case that John had not considered was causing problems in production. He fixed the bug and checked his changes back into source control. And he waited. Hoping for the best.

June spent days writing a software component. She tested and double-checked her code, and she was satisfied that it worked properly, according to the requirements she was given, so she checked it into source control. June's team had a policy that required a code review prior to merging any code with the main branch. During the code review process, one of June's peers pointed out a bug in her code. It was an edge case that June had not considered. She fixed the bug and checked her changes back into source control. The code was reviewed again and merged with the main branch. June slept well that night.

The story of June and John illustrates some of the advantages of code reviews. Catching June's bug during a code review resulted in a faster and cheaper fix and resulted in less public embarrassment than catching John's bug in production. The two bugs were of equal severity, but one was less costly to fix.

What is a Code Review?

Why do we do code reviews? They take up time that could be spent writing code, designing features, or otherwise directly driving forward a project, so there is a cost. The answer is that the benefits of a good code review far outweigh the costs.

When I think of a code review, I think of a formal process in which one person reviews code written by another and provides written or oral feedback to the author, approving that code only after they deem it acceptable.

There are two parties in a code review: The Code Author and the Code Reviewer.

The steps in a code review are:

1. The Code Author makes changes to an application and checks those changes into a code repository
2. The author sends a description of changes to the Code Reviewer. The changes are known as a "Change Set"; the description of those changes is a "Change List"; many Application Lifecycle Management tools (e.g., GitHub and Azure DevOps) support a "Pull Request", which combines the two with the source code and is a formal entry point for the Reviewer to begin reviewing the code changes. I often use these three terms interchangeably because they are so closely related
3. The Code Reviewer retrieves the code, examines it, and (if necessary) provides feedback on changes that the author must make before the code can be merged with the main branch
4. If the code requires changes, the Reviewer sends the feedback to the Author
5. The Author responds to the feedback and makes any necessary changes
6. The Author re-sends to the Reviewer the code with these updates
7. Steps 3-6 are repeated until no more changes are required
8. When no more changes are required, the Reviewer approves the changeset
9. The code is merged with the main branch in the repository

Code Review Goals

A good code review will accomplish the following:

• Validate code
• Help make engineering decisions
• Share knowledge
• Increase code ownership

Let's discuss each of these goals.

Validate code

The most obvious reason to review code is to validate that it does what it is supposed to do. Generally, we look at this from an external point of view. For example, if we provide a given set of inputs to a function, we verify that the function returns the expected output. We can pull the code from source control and make sure it compiles and runs successfully. We can execute automated tests and validate that they all pass.

But we also want to validate the code from an internal point of view. If our team has coding standards, does the code adhere to those standards? While reviewing code, the reviewer looks for and calls out potential problems. Even if the code works well, there may be potential areas for improvement. For example, the reviewer may suggest ways to make the code more efficient, faster, or more readable. The reviewer should point out these as well.

Help make engineering decisions

Sometimes, a code review can drive engineering decisions. If there is confusion or inconsistency about how the application is accessing data or dividing services or testing code, code reviews can raise these issues and prompt a discussion. If different developers have different coding standards, it may indicate a gap in the team's standards and drive discussion around this.

Effective teams have published a set of coding guidelines that may describe everything from naming conventions to required test coverage. Developers must be aware of these guidelines and make an effort to adhere to them; but often non-compliant code slips through. A code review is a good place to catch this before the code is committed to the main branch.

Sharing knowledge

Another benefit of Code Reviews is that it allows sharing of knowledge.

There are two parties involved in a code review process: the code author and the code reviewer. By reviewing the code, the reviewer has a chance to improve the code itself and to address any weaknesses or knowledge gaps in the developer. Similarly, the reviewer can address his or her own weaknesses by seeing someone else's approach to a coding challenge.

The reviewer gains knowledge about a part of the system that someone else wrote. By reading the code, they may also learn something about the language in which it was written; or about a framework or a design pattern or an algorithm implemented by the author; or about the business cases and requirements of the application.

In addition, the code author can learn by reading feedback from the reviewer, who may suggest improvements that the coder did not consider.

Increase code ownership

I have worked on too many systems in which one developer possessed all knowledge about a part of that system. Confusion reigned when that developer left the team. No one understood how to maintain the orphaned code. By conducting regular code reviews, team members have a chance to understand parts of the system on which they are not actively working. This shared knowledge benefits the whole team, allowing flexibility in staffing and removing the danger of all knowledge departing when a team member departs.

Code Review Process

The process of a code review is simple: The author checks code changes into a repository and announces that it is available for review. A reviewer looks at and runs the code and provides feedback. This feedback can be either written or verbal. Most Application Lifecycle Management systems (e.g., GitHub and Azure DevOps) support this process through a Pull Request. In these systems, the code of a Pull Request does not get merged into the main branch until one or more reviewers have approved it. We can configure these systems, setting specific rules about who must approve code before it is merged.

This process works best when everyone involved believes in it and considers code review time to be well-spent. Support from upper management can help encourage the process; but, public buy-in from the team's most respected developers is an even more effective way to get others to buy into this process.

Conclusion

Code Reviews have become an important part of most of the projects on which I work, yet I remember a time before I even knew such a thing existed.

These days, code reviews are almost ubiquitous on my software projects. They help us address weaknesses among developers and reviewers, enforce compliance with coding standards, and improve the quality of our codebase.

If we can catch bugs before they go into production, we can save ourselves embarrassment, time, and money. A good code review process helps us achieve that.