Code Smells: How to Spot Them in Review and Actually Fix Them

Who This Is For

If you're a developer who reviews pull requests and you've ever stared at a diff and thought, "Something's off here, but I can't put my finger on it," this guide is for you. I'm not talking about the obvious bugs—those get caught. I'm talking about the code smells: the subtle structural problems that make future changes slower, riskier, and more painful. The ones that slip through because they're not technically wrong.

I'm going to walk you through my personal checklist for hunting code smells during review, step by step. I've been on both sides of the table—author and reviewer—and I've learned the hard way that ignoring smells leads to a codebase that fights back. So let's get to it.

1. Look at the Overall Design Before Anything Else

When I open a pull request, my first pass isn't about syntax or style—that's what linters are for. The most important thing to cover in a review is the overall design of the change (Google Engineering Practices - What to look for in a code review). I ask: does this change fit the architecture? Is it solving the right problem? A code smell often hides in a design that's over-engineered or prematurely abstracted.

Watch for over-engineering. Google explicitly warns reviewers to be vigilant about code made more generic than it needs to be, or functionality added that isn't currently needed (Google Engineering Practices - What to look for in a code review). I've seen developers build a pluggable plugin system for what was a two-line fix. It's tempting to speculate about future needs, but I've learned to encourage solving the problem we know we have now, not the one we invent for next year.

If the design feels off, that's a smell. Don't rubber-stamp it just because the tests pass. The design is the foundation; if it's crooked, everything built on top will be shaky.

2. Keep Changes Small—It's Not Just About Speed

One of the strongest anti-smell weapons is the size of the change itself. Small PRs are easier to review thoroughly, less likely to introduce bugs, and easier to merge or roll back (Google Engineering Practices - Small CLs). But there's a deeper reason: when a change sprawls across 50 files, the smells multiply and hide. You can't see the forest for the trees.

Google suggests that 100 lines is a reasonable size for a change, and 1000 lines is usually too large (Google Engineering Practices - Small CLs). In practice, I aim for under 400-500 lines of meaningful change, as recommended by Google's code review best practices (Google Engineering Practices - code review). That's not a hard rule—a 200-line change in one file might be fine, but spread across 50 files it's usually too big (Google Engineering Practices - Small CLs).

When I see a monster PR, I immediately suspect code smells: duplicate logic, copy-pasted patterns, and tangled dependencies. I ask the author to split it up by concern. It's not about bureaucracy—it's about making the review actually possible. And when reviews are possible, smells get caught.

3. Hunt for Complexity, Not Just Bugs

Bugs are easy to spot with tests, but complexity is a smell that tests won't catch. I use two metrics from static analysis: cyclomatic complexity, which counts the number of paths through the code (1 + number of conditional branches), and cognitive complexity, which measures how hard the control flow is to understand (SonarQube Server Docs - Understanding measures and metrics, SonarQube Server Docs - Understanding measures and metrics). High complexity means the code is a breeding ground for bugs.

During review, I look for methods that are doing too much. If a function has more than a handful of conditional branches, I start asking questions. Is there a simpler way? Could this be decomposed? A smell I often see is a long chain of if-else statements that could be replaced with a lookup table or polymorphism.

I also watch for duplication. SonarQube tracks duplication as a metric because it's a classic smell (SonarQube Server Docs - Understanding measures and metrics). If I see the same block of logic in two places, I flag it. Duplication means a future change will have to be made in multiple spots, and one of them will be missed.

4. Don't Let Security Smells Slip By

Security issues are the most dangerous kind of smell, and they're not always obvious. I check for input validation, SQL injection prevention, path traversal, hardcoded credentials, and vulnerable dependencies (Google Engineering Practices - code review). The OWASP Top 10 2025 shows that broken access control is the number one risk, and injection is still in the top five (OWASP Top Ten 2025).

I use automated tools to catch the low-hanging fruit. For Python, Bandit builds an AST and runs plugins to find common security issues (Bandit (PyCQA) Docs). For Go, gosec does taint analysis for SQL injection, command injection, and more (gosec - Go Security Checker). But tools aren't enough—they miss the logic-level smells like insecure error handling. The OWASP Top 10 for 2025 added a new category for mishandling of exceptional conditions, focusing on improper error handling and failing open (OWASP Top Ten 2025). That's a smell I now look for with fresh eyes: catch blocks that swallow exceptions, fail-open authorization checks, and exposed stack traces.

One concrete example: I once reviewed a change that added a new endpoint. The code checked that the user was logged in, but it didn't check whether that user had permission to access the resource—a classic broken access control issue. It looked fine on the surface, but a quick question about the authorization flow revealed the smell. Catching that in review saved a potential data breach.

5. Read the Code for Readability—But Not to Nitpick

Readability is a code smell indicator. If I have to struggle to understand what a piece of code does, that's a smell. But I don't nitpick about naming conventions or comment style—that's what linters are for. Instead, I focus on whether the code communicates its intent.

Google says that if a reviewer doesn't understand something, the first response should be to clarify the code itself, not just explain in the review comment (Google Engineering Practices - Handling reviewer comments). Because an explanation in the review tool doesn't help future readers. So when I see a comment that says "// This is tricky," I ask: why is it tricky? Can it be simplified?

I also look for dead code, unused variables, and unnecessary complexity. These are smells that slow down future developers. A good rule of thumb: if a change includes code that isn't used or a dependency that isn't needed, it's a smell. I've even seen unused dependencies slip through because the reviewer was focused on the main logic.

But here's what can go wrong: you can get so caught up in readability that you bikeshed over trivial style points. I've been guilty of this. The key is to label comments by severity—nit, suggestion, blocking—and offer solutions, not just problems (Google Engineering Practices - code review). If it's a nit, don't block the merge. Approve with comments if you're confident the developer will address them (Google Engineering Practices - Speed of Code Reviews). That's the LGTM with comments technique, and it keeps the review moving while still noting the smell.

6. Make It a Habit: Use Quality Gates and Metrics

Code smells are easier to catch when you have objective measures. Static analysis tools like SonarQube can analyze reliability, security, maintainability, coverage, and duplication (SonarQube (GEANT KB)). And a quality gate can block a merge if the code doesn't meet thresholds. The SonarQube 'Sonar way' quality gate, for example, requires no new issues, all security hotspots reviewed, new code coverage at least 80%, and duplication in new code at most 3% (SonarQube Server Docs - Understanding quality gates).

I'm a fan of using such gates as a safety net, but I don't rely on them exclusively. They catch mechanical smells, not the subtle ones that require human judgment. I also use DORA metrics to track our delivery performance: change lead time, deployment frequency, failed deployment recovery time, change fail rate, and deployment rework rate (DORA - Software delivery performance metrics). If my team's change fail rate is climbing, I suspect code smells are accumulating.

Here's what can go wrong if you skip this step: you'll be constantly firefighting. Studies show that peer review increases the number of distinct files a developer knows about by 66% to 150% (Rigby & Bird, ESEC/FSE 2013). That knowledge sharing is what prevents smells from spreading. But if you don't have a structured way to track quality, you'll miss the slow decline.

My Bottom Line

Code smells aren't just about aesthetics—they're about the long-term health of your codebase. The primary purpose of code review is to ensure that code health is improving over time (Google Engineering Practices - Standard of Code Review). So when you review, don't just look for bugs; look for smells that will make future changes harder. Keep changes small, watch for complexity and security issues, and use tools to back up your judgment. And remember: there's no such thing as perfect code, only better code (Google Engineering Practices - Standard of Code Review). Approve a change if it makes the system better overall, even if it's not perfect. But if you spot a smell, flag it—and fix it before it becomes a stink.

Sources

• Google Engineering Practices (code review) - https://google.github.io/eng-practices/review/
• SonarQube Server Docs - Understanding measures and metrics - https://docs.sonarsource.com/sonarqube-server/user-guide/code-metrics/metrics-definition
• OWASP Top Ten 2025 - https://owasp.org/Top10/2025/0x00_2025-Introduction/
• Bandit (PyCQA) Docs - https://bandit.readthedocs.io/en/latest/
• Google Engineering Practices - Speed of Code Reviews - https://google.github.io/eng-practices/review/reviewer/speed.html
• Rigby & Bird, ESEC/FSE 2013 - https://dl.acm.org/doi/10.1145/2491411.2491444