Code review is an increasingly common practice in development teams. It’s a workflow in which developers submit their code for feedback prior to merging branches, or deploying code to production. This feedback is usually given by colleagues, either other developers, a manager, or a tech lead. One of the most familiar forms of code review is the Github pull request, in which developers leave comments on specific lines of code and, ultimately, approve or reject the proposed changes.
Code review is based on the simple assumption that “two heads are better than one”. We all have blindspots when writing code: approaches we don’t consider, efficiencies we don’t make, and parts of the system that we understand less well than others. Code review is an attempt to eliminate these blindspots and improve code quality by ensuring that at least one other developer has input on every line of code that makes it into production.
(As a side-note, pair programming can sometimes resemble a form of ‘live’ code review, where one person writes code and the other reviews it on the spot.)
Though code review often means code takes a little longer to make it into production, many development teams say that it’s worth the time due to an overall increase in code quality. Code review is practiced from massive top performing companies, like Microsoft and Google, to startups like Fullstory. Bruce Johnson, co-founder at Fullstory, says that his company does code review because “an ounce of prevention is worth a pound of cure”. In short, code review often means that fewer mistakes make it into production.
You might already be doing code review at work. However, in my experience, most developers conduct code reviews according to their ‘gut feeling’. They react to each line of code without a clear plan for what they will consider during the code review. Because of this ad hoc approach, certain aspects of code review are often overlooked.
In this article, we’ll aim to build your code review skills by suggesting the different elements you should consider when conducting one. You can use this list a checklist to go through when you’re reviewing code. If this list seems overwhelming, Codementor also offers code review as a service.
1. Readability a.k.a. ‘Understandability’
Readability in software means that the code is easy to understand. In this case, understanding code means being able to easily see the code’s inputs and outputs, what each line of code is doing, and how it fits into the bigger picture. When reading through the code, it should be relatively easy for you to discern the role of specific functions, methods, or classes.
Code becomes less readable as more of your working memory is required to hold each ‘step’ in your mind. One of the most frequent problems with code is that it’s not broken down into small enough chunks. Usually, this leads to classes, methods or functions that are too long with too many tangled responsibilities. By breaking code into smaller chunks, it’s easier to reason about and make changes to specific parts of the system without unintended side effects.
Another aspect of readability is the naming of variables, functions, methods, and classes. Good, descriptive names make code easier to understand. Don’t hesitate to give feedback on names that are overly abbreviated or difficult to understand. For example, while it might be clear to the original coder that op is short for options parser, it may not be clear to you or the next person who will on the code. Good names saves everyone’s time and reduces cognitive load when reading code.
2. Maintainability
One of the most common reasons that code eventually becomes painful to work with is because it isn’t written to be easily to extendable and changeable.
Here are some warning signs that code may not be easy to maintain in the future:
- It’s very tightly coupled to another system.
- Configuration is hard-coded.
- It contributes to tech debt by increasing investment in a technology that the team wants to phase out (e.g., by using functionality from an old version of a library).
- It relies on old code that has been slated for removal or replacement.
3. Security
Security vulnerabilities often enter codebases because developers write code without thinking about security. This might mean that they write insecure code that introduces vulnerabilities into the system, or use libraries and tools that are out-of-date or have known security issues.
During code review, security issues might be overlooked if developers forget to put themselves in the shoes of someone trying to exploit the system. For example, ask yourself: if I was trying to gain access to the system or steal data, how could I exploit this code?
4. Speed and Performance
Consider performance across two dimensions: performance for users and resource consumption. Performance for users reflects a focus on how quickly your code performs for the end user. Lengthy database queries, unoptimized assets, and multiple API requests can all work to make your code feel slow.
When possible, code should use lazy loading, as well as asynchronous and parallel processing. It should use caching as much as possible and shouldn’t load anything that isn’t used.
The other dimension of performance is resource consumption. This means not commissioning cloud servers that are more powerful than needed, not running intensive reports more frequently than needed, and otherwise, not putting the system under more load than it needs to be under as a result of code or infrastructure choices.
While adhering to best practices like these, be mindful not to take this “need for speed” too far. Doing so can lead to premature optimization, which are optimizations that aren’t needed, aren’t noticeable to the user (or in your metrics), or aren’t worth the time investment. Be practical. Focus on the 20% of optimizations that produce 80% of results.
5. Documentation
Check whether the code you’re reviewing requires extra documentation to go along with it. If it’s a new project, this means ensuring it has an adequate readme that explains why the project exists and how to use it. If it’s new code added to an existing project, it’s worth thinking about whether the project’s readme needs to be updated to document the new functionality or new tools.
6. Reinventing the Wheel
Does the code use the right language features to get the job done? When people write code in programming languages they haven’t mastered yet, they often take the long way with code. For example, they might laboriously write out a function to do something that already exists in the language they are using. When doing code review, make sure that the code uses all the appropriate language features. The code shouldn’t re-implement functions that already exist in the language or libraries that the project uses.
7. Reliability
Reliable code is code that is failure tolerant. When things go wrong in reliable code, the user experience is shielded from the impact as much as possible. Reliable code is written on the assumption that things will fail, that assets will sometimes not load, API requests will occasionally return 500 errors, and database records will be missing. When a certain level of failure is anticipated, it can be handled elegantly. Code that assumes nothing will go wrong generally ends up failing catastrophically.
8. Scalability
Consider scalability by imagining what might happen to the code you’re reviewing if it were put under unexpected load. What happens to your homepage if it goes viral and is hit with dozens of requests per second? What happens if a user with thousands of activities in your app decides to view their full activity log? What happens if your product appears in the news and 100 people try to buy it all at once? It’s important to consider what is likely to happen to the code under periods of very high usage when conducting code reviews. After all, the worst time to discover scalability issues is when they take your website/app/service offline.
9. Reusability
Check that the code is written with likely future use-cases in mind. For example, if you’re reviewing code for a marketplace that is rapidly expanding its product range, make sure that the code can easily be updated to support new kinds of products in the future.
By the same token, make sure that the code doesn’t take this too far by trying to account for use cases which are unlikely to eventuate. We’ve all seen code where the author was trying to future-proof their creation so much, that they ended up adding extra features that would never be used to their code. Code that’s never used is immediately legacy code. Therefore, it’s important to strike a balance between code that is reusable and code that violates the YAGNI principle: you aren’t gonna need it.
DRY is one of the first maxims learned by programmers. It means Don’t Repeat Yourself. In other words, don’t duplicate code or functionality. One of the quickest improvements you can make during code review is to identify repetitive code and suggest a reusable function or class to replace it.
A word of caution: it’s possible to take reusability too far and resulting in code that is so abstract and tries to accommodate so many potential use cases that it serves none of them well. It’s the equivalent of trying to invent a kitchen utensil that is a fork, knife, spoon, and plate all in one. It hasn’t been done yet, which is a sign that it’s probably not a good idea!
10. Patterns
Another consideration when adding new code to a codebase is whether it matches the patterns that your team have already established. Your codebase likely already has its own style, and may have a dedicated style-guide. New code shouldn’t deviate from established patterns without good reason.
11. Test Coverage and Test Quality
Code review is as important for tests as it is for the code that is tested. This is because a flawed test is more dangerous than having no test. Passing tests allows the developer to feel secure and willing to push new code to production. But what if one of the tests is passing for the wrong reason, or isn’t testing what it is supposed to test? This kind of test can be a ticking time bomb, allowing bugs to sneak into your codebase.
The same requirements for production code should also apply to tests. Tests should be readable, maintainable, performant, and adhere to established patterns. When it’s time to update or maintain existing code, its tests are likely to be the first thing that needs to change. Therefore, it’s critical that they are easy for your team to work with.
Lastly, don’t stop at reviewing the tests that are there. Think through whether there are tests that are missing. Are there edge cases that haven’t been tested?
12. Fit for Purpose
Code may work, but does it work in the way that your Product Manager, CEO, or the user expects? Before code is pushed to production, it’s worth double-checking that the code actually provides the functionality it was meant to provide. If you don’t have a defined quality assurance process for new functionality, code review may be the only chance you have to confirm this.
13. Notice What’s Missing
Code review can encourage a bias towards considering only what’s in front of you. You review the code that you’ve been given. But what about the code that isn’t there? For example, it’s important to think through edge cases, unexpected inputs, and error handling scenarios that the code’s author may not have considered. What happens when the API that the code relies on goes down? What happens when the user hits the submit button twice in rapid succession? What happens when the user’s browser isn’t supported?
14. Zoom Out
One of the risks with code review is that it encourages a focus on the details of code, rather than the bigger picture. What happens when a pull request is submitted which contains hundreds of lines of code, and yet, the approach to solving the problem is inferior? Perhaps it is inefficient, or brittle, or poorly architected? This can be really difficult feedback to give, especially when the developer has spent several days working on a solution before requesting code review.
However, this kind of feedback is important because pull requests that shouldn’t have been approved in the first place often become pain points in your codebase. You need to be comfortable suggesting a totally new approach if the pull request is fundamentally flawed.
One of the best ways to make this more realistic is to ensure that pull requests are not too big. If developers are working in isolation for days and finally submit a large pull request, this is an anti-pattern. Pull requests should be small and frequently integrated. Feature toggles, sometimes also called feature flags, can help with this. They’re clever tools to enable larger chunks of work to be broken into a collection of incremental pull requests. They allow constant progress on functionality in your codebase without exposing it to users until you’re ready.
Tell us: What else do you consider when reviewing code?
We hope this has served as a useful checklist for you to consider during code review. Even if you don’t refer to every item on the list every time you’re reviewing code, it might be useful to take note of the aspects of code review that you tend to overlook. These will be different for everyone, and will depend on your background or experience. However, all these aspects of code are critical for quality and shouldn’t be skipped.
Looking for a mentor to review your code? Connect with a mentor through our On-demand Code Review Service!
What else do you think is important to consider when conducting a code review? We’d love to hear from you in the comments.
