When done right, code reviews can improve software quality, encourage collaboration, and facilitate knowledge-sharing across your teams. However, common code review mistakes and poor practices can undermine these benefits, ultimately leading to ineffective reviews and wasted effort. Let’s take a look at some prime examples of these common code review pitfalls and how to avoid them.
Focusing too much on style over functionality
One common code review mistake is placing too much emphasis on stylistic concerns rather than functionality. While having a consistent code style is important, a review should focus primarily on logic, architecture, and maintainability. When too much time is spent nitpicking minor style issues, developers can become frustrated and lose sight of the bigger picture.
Example
Instead of constantly pointing out small stylistic issues like inconsistent indentation or missing spaces, ensure you have automated tools in place so reviewers can focus on the bigger picture.
Avoid focusing on the minutiae during reviews:
if(condition) { doSomething(); }
Instead, focus on logic and overall function:
if (importantCondition) {handleCriticalData();logError();} else {handleDefaultData();}
TLDR: Leverage automated tools so you can focus on high-impact areas during reviews.
Not understanding the context of the change
One common code review mistake is diving into a review without fully understanding the change's context. This often happens when a reviewer looks at code in isolation and misses how the change fits into the broader system.
Example
A reviewer may flag a particular function as being inefficient, but without understanding the context, they could miss the fact that this function is only called in an edge case, making its inefficiency less critical.
Reviewer flags this as a performance issue:
for (let i = 0; i < largeDataSet.length; i++) {processData(largeDataSet[i]);}
In reality, this code could be running only once under specific circumstances, so its performance impact is minimal in the grand scheme of the application.
TLDR: Avoid commenting on changes without understanding how the code integrates with the system. Always read the surrounding code and consider the scope of the change.
Overly large pull requests
Reviewing a massive pull request (PR) is a daunting task, and it's easy for reviewers to miss important details when overwhelmed by the sheer size of the code change. A large PR can introduce review fatigue, leading to shallow reviews or ignored sections of code.
Example
A PR that includes a major refactor, the addition of a new feature, and multiple bug fixes can create confusion. Instead, try breaking it up into smaller, logical chunks.
- PR #1:
feat: implement new feature for user authentication - PR #2:
refactor(core): restructure core module for better performance - PR #3:
fix: resolve multiple bugs in user interface and API calls
These follow the conventional commits format of specifying the type (feat, refactor, fix) and providing a concise description of the changes.
Another effective way to avoid large pull requests is by stacking PRs with tools like Graphite. Stacking is a workflow where larger changes are broken down into a sequence of smaller, dependent PRs. Each PR builds on the previous one, resulting in smaller, easier to review, self-contained pieces of code while still maintaining the overall context of the larger change.
TLDR: Don't submit large PRs that overwhelm the reviewer. Instead, encourage smaller, more focused PRs that are easier to digest and review thoroughly.
Not leaving actionable feedback
Another common code review pitfall is providing vague or non-actionable feedback. Comments like "This looks wrong" or "I don't like this" offer no clear direction for improvement. Constructive, actionable feedback is essential to help the author improve the code effectively.
Example
Instead of saying "This code isn't clear," suggest an alternative approach:
"Consider extracting this logic into a separate helper function for better readability, something like handleEdgeCase().
TLDR: Avoid unclear or unhelpful feedback. Aim for specific, constructive comments that help the author understand what to improve and how.
Ignoring testing
A common fault in code reviews is overlooking testing. Code reviews should always include an assessment of how well the code is tested. Gaps in testing can lead to fragile code and bugs going unnoticed. A new feature might be implemented perfectly, but if the reviewer doesn't check that tests have been written (or existing ones updated), it can lead to future regressions.
TLDR: Always ensure there is sufficient test coverage for the changes being reviewed.
Failing to ask questions
Code reviewers are not expected to know every detail about the code they are reviewing, and one of the biggest mistakes is failing to ask questions when something is unclear. Some reviewers feel pressured to appear knowledgeable and may gloss over parts of the code they don’t understand, resulting in missed issues.
Ask clarifying questions Here are some examples to get you started: "Why did you choose this particular algorithm over another?" "Can you explain why we need to loop through the dataset here twice? Is there a specific reason for this approach?" "How easy will it be to extend or modify this function in the future if requirements change?"
TLDR: Ask questions when something is unclear. A good reviewer seeks clarity and understanding before approving changes.
Rushing the review process
Finally, rushing through a code review is a common mistake that can result in missed bugs and other issues. Rushed reviews often happen when deadlines are tight or when the reviewer is managing multiple tasks at once, but this approach can lead to costly errors down the line. For example, a rushed review might approve a feature that includes security vulnerabilities or other bugs simply because the reviewer didn't take the time to scrutinize the code.
TLDR: Slow down. It’s better to spend a little extra time reviewing properly than to have to fix bugs later.
Conclusion
A thoughtful, well-structured code review not only catches bugs early but also fosters a culture of continuous learning and improvement, and avoiding these common code review mistakes will significantly elevate the quality and impact of your reviews. By prioritizing meaningful feedback, encouraging collaboration, and paying attention to the critical aspects of the code, your team can transform code reviews from a mere procedural step into a powerful tool for building better software, reducing technical debt, and enhancing overall engineering productivity.
