Top 8 Code Review Mistakes

Why Code Reviews Matter?

You can't dismiss the significant benefits of code review, whether you're a software development manager or a programmer in the trenches. These evaluations save time, streamline the development process ahead of time, and reduce the amount of work that QA teams have to do later. After all, would a serious book publisher dare to print thousands of copies of an author's work without first reviewing and editing the manuscript by a team of editors? The same logic applies to software authors and publishers. But these days when production cycles are getting shorter and shorter, where do you start?

One way to detect bugs in the generated code and improve its quality is to check the code. Code review is a systematic process of evaluating code generated during the development phase. This code analysis is an ideal way to see the written code with four or more eyes, offering a wide range of points of view and opinions.

Different organizations and people define code review differently. It is not a knitting, fixed process that requires the right format or mechanism. Trial and error is the key to success! Rigorous code review to unleash its full potential can improve the overall quality of a project. This also raises the risk of several errors that you should look out for when viewing the codes. 

Here you can find a list of mistakes that should be avoided during test review.
 

Types of Code Review

There are three main types of code review: bulk code review, post-commit code review, and pre-commit code review. Group code review, the least effective and most intimidating of the three, is when a group of people gets together in a room, a developer displays their code on a large screen and walks everyone through it. There is a double side to this that puts the individual developer in a quandary and allows others to feel ashamed and embarrassing. Both of these actions lead to a decrease in the safety of the work environment, which makes it difficult to achieve trust and cooperation.

It remains to check the code before committing and after committing. Post-commit checkout is a method of checking out an individual code change, usually in the form of a patch, that has been applied to a code repository after it has been checked into that repository. This has the advantage that the code quickly transitions to a deployment-ready state, but has a downside that potentially allows defects to penetrate the repository without being checked. The lack of gating that the post-commit review offers makes it a weaker choice than the pre-commit review, but still much better than the group review, as the post-commit review can be done individually and at the reviewer's leisure. 

Pre-commit code review, using specialized tools such as Gerrit or the Review Board allows you to block a code fix and ensure it goes through the code review process before being added to the code repository. This approach gives you all the benefits of post-commit validation and also ensures that the code is understood and approved by more than one person on your team before it goes into production. I have found that once adopted, this method is the most effective and least likely ignored of the three.

Common Code Review Mistakes

In this section of the article, we will look at the most common mistakes that every code reviewer makes. However, with enough knowledge, these mistakes can be avoided easily.

1. Skipping Tests

This is a common mistake. Refusing tests or skipping tests is one of the greatest mistakes you can make. The review contains a variety of tests, and it may be trivial to review all of them. The process of setting up, approving, and breaking code can get boring over and over again. This repetition pretty soon catches us by surprise and makes us think that the rest of the tests will be perfect. Although it never happens! Several tests elude the code review, putting the entire project at risk. To avoid unnecessary effort, developers usually skim through all the tests and jump straight to the implementations and conclusions.

Tests are also codes that need to be learned to understand how they work and how they function. Ignoring the fact that reviewing tests requires a lot of attention and understanding brings many benefits. For example, finding bugs early in the process is better because the later bugs are discovered, the more expensive they are. 

2. Reviewing Only Newly Added Codes

Code review is an ever-evolving process that goes through various phases of change. During the process, there may be times when certain lines of code might be removed or simply ignored because they might be old. Developers tend to ignore the fact that the generated code is like a well-planned story that needs to be read right away. When you start reading and validating the code piece by piece, you are missing out on some details.

As you read the story, you probably don't like skipping chapters or paragraphs because you might lose concept. It's the same with codes! You should think of codes as a coherent, cohesive story that cannot be broken down or read in chunks. Another common mistake developers do is to go through lines of code that were recently added to the codebase and leave the existing ones.

3. Rush

Reviewing the code correctly takes time and peace of mind. A hasty check can reduce the quality of your code. Minutes before a demo, release, or deadline is the worst time to review your code. In such cases, what usually happens is that the reviewer is just reading lines of code, rather than reading codes line by line. Regardless of anyone's fault, poorly written code was included in the codebase, which would affect the entire project. To avoid this, it is always best to close your work in time before a presentation or demo.

Only if you manage your time and work properly will you have enough time to study changes or fixes. Hasty reviews may even miss minor and minor bugs. Another option that will come to your mind is to use automatic browsing, which will definitely reduce the human factor, but still rushing or hasty browsing is never the best choice

4. Trying To View Too Much Code At Once

Almost everyone makes this mistake.  According to the Smart Bear report, developers should check no more than 200-400 lines of code at a time. After that, the efficiency drops significantly. Work no more than one hour at a time to keep your concentration. Break massive functions with thousands of lines of code into smaller pieces. If you are embarking on a major feature, start doing code reviews at around 30% to see if there are any major flaws and if the approach is correct.

5. Ignoring Design

Another major mistake reviewers make when reviewing code is ignoring their role in the entire process. It is the responsibility of the reviewer to scrutinize the entire code, from design to language, mechanism, and operation. Often, reviewers ignore the design of the code and focus on the functionality and operability of the code. Most of the changes will not affect it, but those that are critical may require careful consideration and observation. In such cases, it is best to consult with other team members and understand their different points of view.

6. Opinion-Based Feedback

This type of feedback usually starts with the words "I wouldn't do this" or "This is interesting." However, the goal is to solve the problem.

Formulate suggestions about when what, and why. Instead of "Never return null when retrieving a list of records from the database," say something like "When returning records from the database, and they are not null, we should return an empty list, because all list methods function with an empty list, and we don't need to include error handling code."

7. Unclear Comments

Removing obscure, obscure comments in the comment section for the author of the code is another serious mistake that reviewers make. It is useless to comment in monosyllables, for example, "Please fix" something that you dislike or disagree with. How does a developer know what you mean? Maybe the developer will figure it out, maybe they don't have enough experience to fix it, or maybe they don't think something needs to be fixed. Before you explicitly state what is wrong with a given snippet, you cannot be sure if the developer understood what you meant by that.

The reviewer should make clear and understandable comments. You must clearly state where and what you think is wrong. To create space for conversation, it is important to leave direct comments. You need to identify problems and come up with a few ideas to create a space for dialogue. Without them, there is no reason for dispute or change of opinion.

8. Bad Timing

In what universe can this be a mistake, you ask? Half an hour before the demo is the worst time to test the code. The correct inspection takes time and peace of mind, and haste cannot guarantee quality. Such situations often arise due to poor management, so programmers do not feel responsible. Regardless of who is to blame, some low-quality code was added to the wrong branch and is likely to remain there.

It's a good habit to close tasks well in advance of a presentation, release, or any other deadline. Nothing else will give you enough time to review and, more importantly, make the necessary changes.