View a tl;dr on Substack
Being able to effectively review code is one of the most difficult tasks to master as a software engineer. Auditing someone else’s code requires an individual to derive and understand someone else’s mental models. To make matters even worse, these mental models are often scarcely explained and riddled with bugs.
As a result, code reviews are often poorly done, or ignored altogether.
This post will cover the general characteristics of a good code review. With this knowledge, I hope that you are better able to contribute to your team & organization by being a helpful & effective reviewer. In future articles, I will be covering strategies that I have discovered through experience to make reviewing code easier and less unpleasant.
Effective code reviews identify potential errors in logic
I’m a firm believer that a code merge is the dual responsibility of the author and the reviewer. Identifying erroneous and/or dangerous code should be the reviewer’s utmost responsibility. Ensure that you have full context over the changes that are being introduced, as some bugs are not apparent unless you have a good understanding of the business logic / software design of the changes. If you don’t have full context, ask a team member that does. Remember, your team is here to help!
Effective code reviews suggest optimizations in logic, efficiency, and readability.
Your goal as a reviewer is to propose better ways of doing things. This could be one of the following:
- Identifying unnecessary logic. For example: “You’ve already done this check above on L21, so we don’t need to do it again here”
- Improving code efficiency. For example: “We can reduce the time complexity from O(n^2) to O(n) by doing X.”
- Making things more readable. For example: “Instead of using a nested ternary, can we split up the conditions to make the logic more understandable?”
Effective code reviews enforce organizational conventions
There are multiple ways to write code, and each of them can be equally correct. Just as authors have their own literary style, developers often write code in slightly different ways.
However, a consistent codebase is more debuggable and comprehensible. As a reviewer, you should understand organizational patterns and identify deviations from those patterns. For example:
- Identifying an existing utility function that could be used instead of an inline implementation
- Highlighting deviations from a team-wide style guide
- Suggesting usage of standardized software design patterns
Effective code reviews expose developer assumptions
As a developer, you create mental models to decompose a complex problem and construct a suitable solution.
As a code reviewer, you often do not have access to those mental models. This is largely why reviews are so mentally draining, but it’s also why external code reviews are so useful to maintaining the readability of a codebase.
A piece of code should be comprehensible to an external reader without execution. This means that the logical flows introduced in a PR should be understandable without you having to run the code. Your job as a reviewer is to expose developer assumptions that have not been communicated properly.
Ask questions as you read the code, such as:
- “What did you mean by the comment on L34?”
- “Why did you use util X instead of util Y?”
- “Was there a reason for adding such extreme retry logic?”
These questions then serve the basis for:
- Uncovering bugs
- Identifying clarifications that should be added as code comments
- Exposing potentially incorrect assumptions that don’t align with the feature being developed
Effective code reviews don’t waste time
One of the biggest mistakes that a reviewer can make is to be overly critical of things that don’t matter. Organizational conventions aside, it is not your job as a reviewer to rewrite the code as if it were your own.
Developers have limited mental bandwidth. If 90% of your comments are nits, then you’re severely reducing the ability of the code author to address the items that actually do matter.
(Not to mention, you’re likely annoying the hell out of the author!)
Being a thoughtful reviewer is one of the most important ways you can contribute to your team and organization. I hope that the insights outlined above can serve as a set of north star guidelines for your next code review.
Top comments (0)