Movatterモバイル変換


[0]ホーム

URL:


Skip to content
DEV Community
Log in Create account

DEV Community

Cover image for Code Review: Best Practices
Raphael Martin
Raphael Martin

Posted on

     

Code Review: Best Practices

Code Review: Best Practices

Introduction

TheCode 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 duringCode Review and how to make it a better process to everyone.


Why Care About Code Review

It's easy to find good reasons for aproject invest in aCode 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 smartCode 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 goodCode 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 inCode Review can be done.

1. Commit Frequently with Small, Atomic Changes

Commits should beatomic. 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:

🚫 AvoidOne commit with several changes
✅ PreferSeveral 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.

🚫 AvoidToo short message
🚫 AvoidCommit message that exceeds the UI limit
✅ PreferCommit 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

BeforeAfter
Image of an App screen before a feature implementationImage 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 thereviewer, 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. Thereviewer should reserve some time to carefully analyze all the info that thecommitter 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:

  • TheCommitter pushes:
funcuppercased(string:String?)->String{ifletstring{returnstring.uppercased()}else{return""}}
Enter fullscreen modeExit fullscreen mode
  • TheReviewer then suggests:> I don't like usingelse in returning functions. Please, change it to:
funcuppercased(string:String?)->String{ifletstring{returnstring.uppercased()}return""}
Enter fullscreen modeExit 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 considerednitpicking. Analyzing the context of the project and the readability of the code also matters when judging if an implementation is good or not:

  • TheCommitter pushes:
funcremoveDuplicates(fromarray:[Int])->[Int]{returnArray(Set(array))}
Enter fullscreen modeExit fullscreen mode
  • TheReviewer then suggests:> Creating aSet from anArray does not maintain order and may not be the most efficient approach. I suggest you changing it to something similar to:
funcremoveDuplicates(fromarray:[Int])->[Int]{varseen:[Int:Bool]=[:]returnarray.filter{seen.updateValue(true,forKey:$0)==nil}}
Enter fullscreen modeExit fullscreen mode

In the example above, even though theReviewer 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 morereadable 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 thecommitter. 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:

classSettings{privateletdefaults=UserDefaults.standardvarisDarkModeEnabled:Bool{get{returndefaults.bool(forKey:"isDarkModeEnabled")}set{defaults.set(newValue,forKey:"isDarkModeEnabled")}}}
Enter fullscreen modeExit 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 toDependencyContainer.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 thecommitter and thereviewer 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 inCode 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 modeExit fullscreen mode

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

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

2. Create checklists

Another excellent idea, that could be combined withCreating a useful PR template, is creating checklists. You can define a set of tasks for both thecommitter and thereviewer to be performed before doing their "part of the job" in aCode 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 modeExit fullscreen mode

3. Automation

We previously covered that thereviewer 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 aGit 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 thereviewer focus their energy in what really matters: the logic, perfomance and safety of the code.


Encouraging a Positive Code Review Culture

Creating a positiveCode 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 acommitter, you can face the reviews in your code as an opportunity for learning, being thankful to good suggestions received.
As areviewer, 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-structuredCode 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, bothcommitters andreviewers can make the process more efficient and beneficial for everyone.

Ultimately, the goal ofCode 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 turnCode Review into a rewarding experience rather than a bureaucratic hurdle.

Top comments(0)

Subscribe
pic
Create template

Templates let you quickly answer FAQs or store snippets for re-use.

Dismiss

Are you sure you want to hide this comment? It will become hidden in your post, but will still be visible via the comment'spermalink.

For further actions, you may consider blocking this person and/orreporting abuse

iOS & tvOS dev into Swift, SwiftUI, Combine, and HLS. Crafting smooth, responsive apps with great UX. Love pushing tech boundaries to build apps that can handle anything thrown their way.
  • Location
    🇧🇷 Brazil
  • Work
    Senior iOS/tvOS Software Engineer
  • Joined

More fromRaphael Martin

DEV Community

We're a place where coders share, stay up-to-date and grow their careers.

Log in Create account

[8]ページ先頭

©2009-2025 Movatter.jp