Code Review: Best Practices

Raphael Martin - Feb 28 - - Dev Community

Code Review: Best Practices

Introduction

The Code Review process is an important part of the software development cycle, followed by most of the companies nowadays. The idea of having your code reviewed by a colleague or a lead can create mixed feelings in a developer: enthusiasm to be congratulated to a great work, or receive constructive feedback that will enlighten your knowledge, but also insecurity to have your work judged. Today we're going to see common challenges during Code Review and how to make it a better process to everyone.


Why Care About Code Review

It's easy to find good reasons for a project invest in a Code Review process in its development flow, but which are the advantages to you, the software engineer? Maybe you are facing it now as "one more bureaucratic step" in your daily work, but caring about making the process better could be a benefit for you:

  1. Make it easier for the Reviewer: Having a structured and smart Code Review process makes it easier and faster to the reviewer (that can possibly be you).
  2. Get PRs approved faster: If reviewing is an easy and fast task, your code will be approved faster, improving also the velocity that you deliver work to the end-user.
  3. Receive better feedback: A good Code Review process allows the reviewer to focus their energy in analyzing your code, giving better feedback, avoiding losing time with unimportant tasks.

Writing Code That’s Easy to Review

Pushing code that is easy to be reviewed is not so different from writing high quality code. Focusing on clarity, minimizing complexity and consistency are key values that you should follow. However, some specific actions focusing in Code Review can be done.

1. Commit Frequently with Small, Atomic Changes

Commits should be atomic. That means that one commit should be as small as it can, indivisible, in a way that each one makes sense on its own. Frequent commits help create a rich history of the implementation. That way, the reviewer has the possibility of reviewing isolated changes, commit by commit, understanding better your reasoning process:

🚫 Avoid One commit with several changes
✅ Prefer Several atomic commits

2. Rich Commit Messages

Provide a brief, descriptive title and a detailed commit message. The title should be concise, while the message can include specifics about the changes made.

🚫 Avoid Too short message
🚫 Avoid Commit message that exceeds the UI limit
✅ Prefer Commit message with a brief title and a collapsible and rich description

3. Visual Enhancements

3.1 Rich Screenshots

It's not uncommon that a reviewer doesn't have a deep understanding of what the feature you're working on is. Our code may be obvious to you but not to others. That said, adding evidence of what your change does in the software is a good way of giving context. Use visual elements like arrows and highlights to improve clarity.

Implemented fallback image for articles without image

Before After
Image of an App screen before a feature implementation Image of an App screen after a feature implementation, pointing where the change is

When static images are not enough, add videos to your PR description. Prefer them over GIFs, since with videos the reviewer can control the playback, making the understanding of the change better.

3.2 Committing IDE-handled files

Files that are managed by a software often are not human-friendly. And sometimes, we need to add them to a VCS. In Apple development with Xcode, for example, you need to add .pbxproj files to version control, and for the reviewer sometimes is hard to understand what a change in this file is:

Changes in IDE-handled file, hard to be read

It's recommendable adding screenshots of the IDE UI, indicating which changes were made that resulted in the file change, making the review process easier.

Screenshot of the IDE, indicating where the change was made


How to Review Code Effectively

In the role of the reviewer, there are several tips in how to make the process more efficient, positive and peaceful.

1. Read the PR description carefully

In the last section we talked about how to make a good PR to the reviewer, and you can notice that it takes some effort. The reviewer should reserve some time to carefully analyze all the info that the committer added to the PR in order to avoid mistakes. A detailed PR description is useless if the reviewer doesn't read it carefully.

Engineer performing a code review

2. Avoid Nitpicking

In software engineering is very common having several ways to do the same thing, and engineers usually have their preferences. Sometimes, the committer preference is different from the reviewer. If it's not something that violates a project pattern/convention, there's no need to ask it to be changed, example:

  • The Committer pushes:
func uppercased(string: String?) -> String {
    if let string {
        return string.uppercased()
    } else {
        return ""
    }
}
Enter fullscreen mode Exit fullscreen mode
  • The Reviewer then suggests: > I don't like using else in returning functions. Please, change it to:
func uppercased(string: String?) -> String {
    if let string {
        return string.uppercased()
    }

    return ""
}
Enter fullscreen mode Exit fullscreen mode

There's nothing wrong with the initial implementation, and the suggested change is just a matter of preference.

But not only equivalent code can be considered nitpicking. Analyzing the context of the project and the readability of the code also matters when judging if an implementation is good or not:

  • The Committer pushes:
func removeDuplicates(from array: [Int]) -> [Int] {
    return Array(Set(array))
}
Enter fullscreen mode Exit fullscreen mode
  • The Reviewer then suggests: > Creating a Set from an Array does not maintain order and may not be the most efficient approach. I suggest you changing it to something similar to:
func removeDuplicates(from array: [Int]) -> [Int] {
    var seen: [Int: Bool] = [:]
    return array.filter { seen.updateValue(true, forKey: $0) == nil }
}
Enter fullscreen mode Exit fullscreen mode

In the example above, even though the Reviewer is right about the initial code not keeping the array order and possibly having some overhead (in huge arrays), maybe for the purpose of where it's being used the order is not important, and no differences can be seen by the user using one or another algorithm (even though the second one is really faster, only a benchmark test could say that). If that's the case, keeping a more readable code is more important for the project in general rather than an insignificant perfomance improvement.

3. Prioritize doing reviews

Working with global teams in different timezones is becoming more and more common. It's very likely that a coworker that lives in the other side of the globe, sends a Pull Request to be reviewed by you in a moment that you are not working, and that's ok. But since you have a chance, do the pending reviews as soon as possible. Your colleague may be under pressure due to an urgent deadline, and having a PR reviewed fast can save his day.

4. Add references to your suggestions

A suggestion that seems obvious to you might not be clear to the committer. Adding links to documentation, or even pointing another place in the current project that has a similar implementation can bring a better understanding of what your suggestion is:

Given code:

class Settings {
    private let defaults = UserDefaults.standard

    var isDarkModeEnabled: Bool {
        get {
            return defaults.bool(forKey: "isDarkModeEnabled")
        }
        set {
            defaults.set(newValue, forKey: "isDarkModeEnabled")
        }
    }
}
Enter fullscreen mode Exit fullscreen mode

🚫 Avoid a comment like:

You could use a property wrapper for that.

Prefer:

What about using a property wrapper for that? https://docs.swift.org/swift-book/documentation/the-swift-programming-language/properties/#Property-Wrappers. You can also refer to DependencyContainer.swift:37 where we use a property wrapper to inject dependencies.

5. Praise good work

Code reviews aren't just for finding flaws—they're also a great opportunity to recognize good work. Sometimes a committer implements a very robust solution for a complex problem, and you identify that it was very clever. Don't lose the opportunity to praise them about it. This behavior creates a positive environment.


General Tips For A Better Process

We already covered best practices that the committer and the reviewer should follow. What about the process in general, things that affect both of the "roles" in this process?

1. Create a useful PR template

The quality of a Pull Request template can make the difference in Code Review. A bad one can be bureaucratic, inconsistent and hard to be read:

## Describe your change
<!-- Add your change description here -->

## Module affected
[ ] Home
[ ] Settings
[ ] Backend services
Enter fullscreen mode Exit fullscreen mode

Imagine that your change doesn't apply to any of the three modules present in the PR template. The committer will have the work of either removing this section, or just ignoring it. This last option creating a polluted PR description, which will make the reviewer task harder, not easier.

PR templates should align with the project's reality, helping the committer create a rich description in a easier way, and not the opposite.

2. Create checklists

Another excellent idea, that could be combined with Creating a useful PR template, is creating checklists. You can define a set of tasks for both the committer and the reviewer to be performed before doing their "part of the job" in a Code Review process, and then add it to the PR template. Example:

**Commiter Checklist**
[x] Updated the CHANGELOG
[x] Added documentation to public API
[x] Passed the automated UI tests locally
[x] Moved the JIRA ticket to "In Review"

**Reviewer Checklist**
[ ] The committer checked the checklist properly
[ ] This PR follows the [Project Code Standards](repoaddress.com/code-standads-location)
[ ] Created Unit Tests

Enter fullscreen mode Exit fullscreen mode

3. Automation

We previously covered that the reviewer should avoid Nitpicking. A good strategy to make it easier is to create automations for repetitive and mechanical tasks, such as formatting, styling standards and more.
Setting up a Git Hook that runs a linter, for example, can avoid that the reviewer reject a PR due to a bracket opened in the wrong line.
You can also setup automated pipelines to run code checking for bad smells or even to run unit tests.
Automating all the no-human part of the process gives more free time to the reviewer focus their energy in what really matters: the logic, perfomance and safety of the code.


Encouraging a Positive Code Review Culture

Creating a positive Code Review culture in a team is not easy. But you should be an actor that contributes to making it a valuable part of your project.
As a committer, you can face the reviews in your code as an opportunity for learning, being thankful to good suggestions received.
As a reviewer, you can use the review time to reflect about your project features, creating solid understanding of all parts of the software you work with, such as learning new stuff from good implementations of the other devs in your team.


Conclusion

A well-structured Code Review process is more than just a quality checkpoint—it’s an opportunity for learning, collaboration, and continuous improvement. By writing code that is easy to review, providing clear and detailed PR descriptions, and approaching reviews with a constructive mindset, both committers and reviewers can make the process more efficient and beneficial for everyone.

Ultimately, the goal of Code Review is not just to find mistakes but to build better software together. A positive and thoughtful review culture fosters knowledge sharing, strengthens team relationships, and enhances code quality in ways that benefit the entire project. By following these best practices, you can help turn Code Review into a rewarding experience rather than a bureaucratic hurdle.

. . . . . . . .