Skip to main content
Code Smells

Stop Hunting for Smells: Review for Design, Not Perfection

Code smells aren't the real problem. You're wasting review time on nits while the big picture—design, clarity, and over-engineering—goes unchecked. Here's how to fix it.

You've been told to hunt for code smells. Long methods, god classes, duplicated logic—these are the supposed enemies of clean code. But here's the contrarian truth: most code smells are symptoms, not the disease. The disease is unclear thinking, and no amount of scent-tracking will cure it.

I'm not saying code smells don't matter. They do. But if you're spending your review cycles flagging every if nesting or magic number, you're missing the point. Google's own engineering practices say the primary purpose of code review is to make sure the overall code health of the codebase is improving over time (Google Engineering Practices - Standard of Code Review). That's a design-level goal, not a line-level one.

Design First, Smells Second

When you open a pull request, what's the first thing you should look at? The design. Google's review checklist puts overall design at the top—before functionality, complexity, tests, naming, and comments (Google Engineering Practices - What to look for in a code review). Why? Because a well-designed change with a few minor smells is still a good change. A poorly designed change with perfect formatting is a disaster waiting to happen.

Let's be blunt: if you're flagging a missing else branch while ignoring that the whole module is a god object, you're not doing your job. The god object is the real problem—it's going to haunt the next ten developers who touch it. The missing else is a two-second fix.

The Over-Engineering Trap

One of the biggest code smells isn't in your static analyzer's rulebook. It's 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). This is the smell that silently kills codebases, and it's invisible to tools.

Let me give you a concrete example. Say you're reviewing a function that fetches a user's profile. The author wrote it to accept a format parameter that can return JSON, XML, or even a custom binary format—because "we might need it later." That's a pure speculative generality. Your job as reviewer is to push back: solve the problem you know needs solving now, not the one you speculate about (Google Engineering Practices - What to look for in a code review).

This is where the traditional smell-hunting fails. A linter won't catch over-engineering. Cyclomatic complexity metrics might flag the extra branches, but they won't tell you the feature is unjustified. Only a human reviewer can make that call.

The Counterargument: Smells Are Cheap Heuristics

You might argue: "But smells are easy to spot and fix. They're a cheap way to catch problems early." I hear you. And I'd agree—if your reviews were only about correctness. But research shows that finding defects is not the primary focus of modern code review. A study at Google found that the key expectations are education, maintaining norms, gatekeeping, and accident prevention—not bug detection (Modern Code Review: A Case Study at Google (ICSE-SEIP '18)).

So if you're only looking for smells, you're optimizing for the wrong outcome. You're playing whack-a-mole with symptoms while the real issues—design clarity, unnecessary complexity, poor knowledge sharing—go unaddressed.

That's not to say you should ignore smells entirely. Use your linters and static analyzers to catch the mechanical stuff. Google's guidance is clear: let automation handle style and mechanical checks so human review focuses on logic and architecture (Google Engineering Practices - code review). Tools like SonarQube can give you a maintainability rating based on technical debt ratio—A for under 5%, E for over 50% (SonarQube Server Docs - Understanding measures and metrics). But remember: a low technical debt ratio doesn't mean the design is good. It just means the code is conventionally clean.

Practical Advice: What to Actually Look For

So what should you focus on instead? Here's my blunt recommendation, based on Google's checklist and the research:

  • Design: Does this change fit the existing architecture? Is it the right approach?
  • Clarity: Would a new developer understand this in six months? If someone says they don't understand your code, clarify the code, not just the comment thread (Google Engineering Practices - Handling reviewer comments).
  • Tests: Are they meaningful? Remember, tests don't test themselves (Google Engineering Practices - What to look for in a code review).
  • Over-engineering: Is this solving a real problem or a hypothetical one?

And here's a hard number to keep in mind: at Google, the median change size is just 24 lines (Modern Code Review: A Case Study at Google (ICSE-SEIP '18)). That's tiny. If you're reviewing a 500-line PR, you should be asking why it's so big—not because size is inherently bad, but because smaller changes are reviewed more quickly and thoroughly (Google Engineering Practices - Small CLs). A 200-line change in one file might be fine, but spread across 50 files it's usually too much (Google Engineering Practices - Small CLs).

So stop hunting for smells like a bloodhound. Start reviewing like an architect. Your codebase will thank you.

The One Thing to Remember

The most important thing you can do in a code review is to evaluate the design and overall code health—not to nitpick every minor smell. Remember Google's senior principle: there's no such thing as perfect code, only better code (Google Engineering Practices - Standard of Code Review). Approve when the change improves the codebase, even if it's not perfect. That's the real goal.

Sources

  • Google Engineering Practices - Standard of Code Review - https://google.github.io/eng-practices/review/reviewer/standard.html
  • Google Engineering Practices - What to look for in a code review - https://google.github.io/eng-practices/review/reviewer/looking-for.html
  • Google Engineering Practices - Small CLs - https://google.github.io/eng-practices/review/developer/small-cls.html
  • Modern Code Review: A Case Study at Google (ICSE-SEIP '18) - https://www.papercache.org/papers/mlsys/system/2026/03/25/modern-code-review-a-case-study-at-google
  • SonarQube Server Docs - Understanding measures and metrics - https://docs.sonarsource.com/sonarqube-server/user-guide/code-metrics/metrics-definition

Share this article:

Comments (0)

No comments yet. Be the first to comment!