Bienvenido dear subscribers! Glad to be back with yet another episode of your favourite Biweekly Engineering! 🚀 

In today's [special] episode, I want to share some valuable lessons I've learned over the past couple of years on code review. These insights come straight from my experience in the software engineering industry. So go ahead, take a breather, and see how they resonate with your own journey!

No time to waste, let's dive right in!

What is code review again?

Code review is a widely practiced technique in the tech industry, where code written by one person undergoes review by at least one other individual. The purpose is to ensure the correctness of introduced changes and adherence to the codebase or company standards. Have a fresh pair of eyes examine your modifications, understand your intentions, and provide valuable feedback for code correction and improvement. Ultimately, the aim is to obtain approval from the reviewers to merge and deploy your changes into production.

I'm pretty sure most, if not all, of my readers are familiar with the concept of code reviews. So let’s move on.

When exactly is code review effective?

When code review is integrated as an integral part of the development process to harness its benefits, we can deem it as effective. The goal is not to conduct code reviews merely for the sake of it, but rather to derive value from the practice.

Now, the question arises: how can we determine if our review process is indeed effective? Look out for the following indicators within your team:

• When you submit a merge request (MR) for review, there should be individuals available to review the request within a day or two at the very least. If this is not the case most of the time, it suggests a possible sluggish pace within your team that may require attention.

• Meaningful feedback and engaging discussions typically emerge from the MRs, involving active participation from team members.

• It should be rare to see MRs being approved without any comments. Most MRs should elicit some form of feedback or suggestions.

• Review comments provided by the reviewers should be acknowledged and addressed as necessary.

• Reviewers should not unnecessarily adopt a strict stance when approving an MR. They should exhibit consideration and be fully aware of the significance of the MR at hand.

• The review process occasionally uncovers bugs, anti-patterns, or code smells in submitted merge requests. Remember, even with a team of 10x engineers, humans are prone to errors.

• Reviewers should possess the minimum required context regarding the changes made in the MR. Sometimes, your MR might be reviewed by someone who lacks familiarity with your specific code changes. To mitigate this, it is advisable to provide a concise and focused explanation of the changes in the MR description. We will discuss more about this later.

• MRs submitted for review should not be excessively large. Large MRs often leave reviewers struggling to grasp the full context, which is why smaller or moderately-sized MRs tend to be more effective and efficient.

The aforementioned points serve as indications of an effective code review process. If you find that most of these elements are not evident within your team or company, it's time to take some initiatives!

How to do effective code review?

In this section, I will outline some key principles for carrying out effective code reviews. As mentioned earlier, these guidelines stem from my personal experience as a software engineer. While many of these ideas may not be entirely novel to you, it's always beneficial to revisit and reinforce them.

Let’s discuss.

Have proper context

• Before reviewing someone else's code, ensure that you have the correct context of the changes first.

• If the context is not obvious, synchronise with the developer who submitted the changes and build up the understanding.

• If the code is overly complex or highly critical, consider conducting a live call with the developer to review the code. However, based on my experience, such scenarios are relatively infrequent.

Accept multiple MRs instead of one large MR

• Reviewing requires engineers' bandwidth. Large MRs can significantly slow down the review process and increase the likelihood of overlooking mistakes. If you are asked to review a lengthy MR, consider requesting the possibility of breaking it down into multiple smaller MRs.

• As a reviewer, I have always found it easier to navigate through shorter MRs and provide valuable feedback.

• It is quite common to see lengthy MRs being approved with fewer questions. This could be because the reviewers didn't have sufficient time or energy to thoroughly review the entire MR. To facilitate effective code review, it is preferable to have shorter MRs.

Have context in the MR description

• As a developer, include the context of the MR in the description.

  1. Answer questions such as:

     • Why were the changes made?

     • What do we aim to achieve with the changes?

     • What impact does it have in production?

  2. Writing comprehensive descriptions in MRs allows reviewers to easily access the context when checking the code.

• The length of the description depends on the content of the MR.

  1. For minor updates like upgrading a library version, a concise description like "This MR upgrades library-x from version 0.9 to 1.0" would suffice.

  2. However, if the MR introduces a new library, provide an explanation for its necessity.

• As a reviewer, request an MR description.

  1. Not only does this help you understand the context, but it also serves as a documentation.

  2. When someone encounters the change in the future, they can immediately understand why specific changes were made and what they accomplished.

The practice of writing proper MR descriptions is a great way of documentation. Unfortunately, this practice has become rare in many cases, especially due to the fact that we developers don't prioritise documentation.

Ensure clear commit message

• A commit message should provide a clear summary of what the MR contains.

• If an MR lacks a clear commit message, suggest updating it to accurately capture the intention.

• Avoid approving MRs with generic commit messages like "fix a bug". The message should specify what the bug was, such as "fix the null pointer exception bug caused by an unhandled error in the /v1/get/some/data API".

• Remember that a commit message is not meant to capture the details of the MR. It should serve as a concise summary of the MR's essence.

• The commit message is complemented by the details provided within the MR as the description. This is another reason why description is super important.

Encourage industry and company standards

• As a reviewer, it is essential to be familiar with the company and industry standards pertaining to the programming language and framework being used. This knowledge enables you to provide detailed feedback on the changes and ensure high code quality in the codebase.

• If you are unfamiliar with the standards for the language or framework, emphasise the importance of adhering to those standards to prevent the developer from inadvertently overlooking them.

• When it comes to programming languages, it is always advisable to adopt industry standards for fundamental practices. For instance, in C++, variables are typically written in snake_case, while in Go, camelCase or MixedCase is commonly used. It wouldn't make much sense to deviate from these conventions.

Check for readability, maintainability, and reusability of the code

This is one of the crucial aspects of code review. Keep in mind the common best practices while reviewing for readability, maintainability, and reusability of the code.

• Functions should be building blocks of the code. Ask questions like

  1. Are there repeated code blocks? Then the repeated code should be written as a function and used everywhere as a function call.

  2. Are there conditional checks inside if else blocks that are too long? Then these checks should also be written as functions.

  3. Is the function too long? Then it should be broken into multiple functions. Some companies enforce conventions like “a function should not be longer than 20 lines.”

• Code should be modular.

  1. Ensure the use of modular coding style where relevant classes and functions are separated in modules or packages.

  2. Modularisation increases the maintainability and reusability of the code to a great extent.

• Check for the use of single-responsibility principle (SRP).

  1. A class, method, or module should have single responsibility. This principle promotes code clarity and makes it easier to understand, modify, and reuse components.

  2. SRP helps to reduce confusion for a developer working on the codebase for the first time.

• Variables should not be named or used loosely.

  1. Global variables should be avoided as much as possible. It is always a better idea to have local variables and pass them around as necessary.

  2. Variable names should be meaningful, even if it is a bit verbose. But preference should be for concise and meaningful name.

  3. In case of a short scoped variable, short names could be okay. For example, in Go, it is fairly common to see code like for k, v := myMap where k and v stand for a key and value in the map. In scenarios like this, refer to the company standards and follow them. If there are non, follow the practices in the community.

  4. If the value of a variable is never meant to be changed, declare it as a constant. This prevents accidental assignment of the variable in the later part of the code, and also constant variables are thread-safe.

• Ensure that the overall coding structure of the project is followed in the review. For instance, if the project is written using OOP, the new changes in the MR should also adhere to the structure by introducing new classes and methods, instead of adopting a completely different approach like functional programming.

• Verify that errors are handled properly in the MR. Often, unhandled errors can cause exceptions in the running process, leading to program termination. Errors should be caught and handled gracefully, although they can sometimes be easily overlooked.

• Keep an eye on the observability of the changes being made. Ask questions like:

  • Does the code have proper logging in place?

  • Should the changes introduce new metrics to be reported to an external system?

• Ensure that different parts of the code are placed where they are intended to be. For example, if there is a JSON file containing configurations, it should be located within a designated config file or section.

• Check for formatting inconsistencies. Formatting is typically well-defined within the codebase by the team or company. It is common to see automated tools that check for formatting errors and enforce them.

Keep in mind that code written in a readable, reusable, and maintainable way is also future-proof. It enhances the extensibility of the code which is one of the core principles of software engineering: make things extensible.

Encourage testing and testability

• Writing unit and integration tests is crucial. As a reviewer, emphasise the criticality of having tests and encourage others to write them.

• Tests don't need to be included with the actual changes. It is acceptable to have separate efforts for writing tests. However, having tests in the same MR as the code changes can aid reviewers in understanding how the functions will be used. If the MR is not too big, include tests.

• Given that tests are important, it is vital to have testable code. There are common strategies to achieve testable code:

  1. Write code as modules and functions.

  2. Use the dependency injection principle where dependencies of a function or an object are injected in them instead of having them created inside.

Be respectful and considerate while providing feedbacks

• Avoid rude or hostile language while making comments on an MR. There is really no benefit in it.

• Have a respectful tone. If your language sounds harsh, that might negatively impact the environment in the team and harm the productivity of the developers.

Check for correctness

• Lastly, let me mention the obvious. As a reviewer, you need to check the correctness of the code and catch any potential bug.

• In case of complex and highly critical code, consider reviewing the code with other reviewers. This helps to catch any potential issues more easily.

Not every company or team has (effective) code reviews

Surprisingly, I have noticed that not every company has a code review practice, and even when they do, its effectiveness is not always guaranteed.

This is particularly common in startups, where prioritising product delivery often takes precedence over code quality. Additionally, some companies may lack the necessary resources to implement a comprehensive company-wide code review practice.

Unfortunately, there are also instances where certain companies simply disregard the importance of code reviews altogether.

Code review is costly

Guess what? Code review doesn't come for free. It entails two aspects that come with a cost for you and your team:

• Time: Code review requires a significant amount of engineering time. When you submit a Merge Request (MR), reviewers need to allocate a portion of their bandwidth to review your code. A substantial amount of engineers' time is spent on code reviews.

• Speed: Development speed is reduced. The purpose of code review is to ensure the correctness and quality of the introduced changes. This means you can't deploy new updates without approval from the reviewers. Consequently, changes cannot be immediately deployed into production once development is completed. This significantly slows down the process at times.

Concluding remarks

Despite the potential downsides of code reviews, the benefits they offer far outweigh any disadvantages. This is evident from the widespread adoption of code review practices in the industry, with established companies embracing and enforcing their own standards.

It comes as no surprise that code reviews have become an integral part of software development processes. They serve as a crucial mechanism for ensuring code quality, fostering collaboration, and driving continuous improvement. By leveraging the collective knowledge and expertise of a team, code reviews contribute to the overall success of a project.

So, embrace the power of code reviews and recognise their value in enhancing code quality and promoting best practices. Make code reviews an integral part of your development workflows (if you are not doing it already) and reap the benefits they offer. :)

And that concludes today's episode! I hope you found the discussion enjoyable and informative. I certainly enjoyed writing it as well.

Please keep in mind that the points covered in this discussion are not exhaustive. There is undoubtedly more to code review than what has been discussed so far. However, I hope that you have gained a good understanding.

Thank you for reading, and I look forward to seeing you again soon! 👋