Mastering Code Reviews: A Comprehensive Guide

Once a developer has finished the job of dealing with an issue, another developer takes a look at the code and considers a variety of aspects. The process is referred to as code reviews and the person performing this task is the code reviewer.

The basic purpose of code reviews is to ensure that all the code is healthy and they are improving through constant evolution. This goal is achieved by using a variety of processes and tools that are specifically designed for this purpose.

While reviewing the code, a reviewer looks for obvious logic errors, implementation of essential cases, the efficacy of existing automated tests for the new code, and more.

We are taking a look at the best practices for code reviews followed by developers. Some 0f these best practices are followed for code reviews at Google!

What are Better Code Reviews?

These types of code reviews help engineers to continue improving their ways of doing code reviews. While performing such reviews, the developer pays attention to the code change concerning the codebase.

Better code reviews customize their approach depending on the situation and context. Apart from ensuring high-quality code reviews, such practice also helps the developers to enhance the productivity of the code reviews themselves.

Things to Consider when Performing Code Reviews

Here are a few of the aspects to analyze in code reviews. However, when considering each of these, make sure to follow the best possible approach.

Functionality                  

Code reviewers take into account the functionality of code. For instance, reviewers analyze whether the code performs the way its developer intended. Normally, it’s hard to judge how the changes will affect the functionality by merely reading the code.

To deal with this issue, a reviewer might ask the developer to give a demo. Furthermore, functionality also matters when some kind of parallel programming is involved. This might result in race conditions or deadlocks.

Design

No doubt, the user interface (UI) and user experience (UX) design is one of the most important factors to cover while doing code reviews. Being a reviewer, you have to judge whether the interactions between different pieces of code make any sense. Moreover, the design must integrate with the other elements of the system in the desired manner.

Complexity

The CL shouldn’t be overly complex. For instance, over-engineering is a specific type of complexity. Mostly this happens when the developer adds an unnecessary functionality. Therefore, developers should be encouraged to solve the prevailing issues and not the ones that are merely based on speculation.

Code Comments

The reviewers must look at the comments written by developers. Such comments must be written in understandable English. In addition, unnecessary comments should also be removed. A developer only needs to add a comment if the code isn’t clear at some point.

Comments vary due to the documentation of classes, functions, or modules. It needs to provide the purpose, usage, and behavior of code.

Consistency

In most cases, the existing code must show consistency with the style guide. The CL should meet the requirements mentioned in the guidelines. On the other hand, if the style guideline isn’t a requirement, its consistency with the style guide isn’t of utmost importance.

Prefer Small Changes Over Large Ones

The author of the code is responsible for submitting the Change Requests (CRs) that are easy to review. This tries not to waste the time of reviewers. Furthermore, small changes are much better as compared to bigger ones.

If a reviewer makes changes to ~5 files or more, it would be a better idea to split the review into multiple short reviews. Similarly, changes must have a well-defined, narrow, self-contained scope.

The ‘behavior-changing’ changes must avoid refactoring. On the contrary, refactoring changes shouldn’t change the code behavior. The most prominent reasons for this are given below:

• The time of a reviewer must be used for programming the logic and not on style, formatting, or syntax debates.
• It’s hard to undo a behavior change, which was made as an integral part of a refactoring change.
• Such changes usually touch multiple files and lines, as a result, the reviewer would pay less attention.

Code Reviews Procedure

It is extremely important to ensure good quality CL. This means the overall code health of the codebase won’t decrease as time passes. Usually, codebases experience degradation, as the code health continues to decrease little by little.  

More specifically, when under consistent pressure, the developers try to look for shortcuts to meet their goals. In addition, reviewers are responsible for the code they are reviewing.

Therefore, each reviewer must try to ensure that the consistency of the codebase isn’t compromised at any cost. Code reviews also demand that the codebase stays maintainable. 

A CR is considered as the synchronization point between various team members. Hence, the chances of blocking progress are fairly high. As a result, the code reviews should be prompt and the team members need to be aware of their commitment.

If a reviewer isn’t capable of reviewing a code within a given time, it would be better to pass it to other reviewers. Furthermore, a reviewer should explain the change thoroughly. Enforcing the coding standards is also the responsibility of the reviewer.

In the case of a conflict regarding CR, the developer should find a way to reach a consensus. When it’s hard for both the developer and the reviewer to reach an agreement, a face-to-face meeting should be a viable way to resolve the conflict.

If it isn’t possible to deal with the situation, broaden the discussion to include other team members. The reviewer must think about involving Eng. Manager or any other experienced member.

Final Words

With a view to achieving the best possible outcomes through code reviews, it is essential to balance a variety of trade-offs. The developers must be in a position to diligently work on their tasks.

The CR is a way to improve the codebase. However, if a reviewer fails to keep things simple and easy to understand, developers won’t be in a position to improve their coding in the future.