Smells and Heuristics

Chapter 17 of Clean Code is the book's own reference list: dozens of numbered code smells and heuristics, grouped by category, that Martin compiled while refactoring real programs. It is meant to be read once top to bottom and then used as a lookup table. Condensed here for quick recall - one line per smell, grouped exactly as the book groups them.

Comments

• C1 - Inappropriate information: don't put author names, change history, or ticket numbers in a comment - that's what source control and issue trackers are for.
• C2 - Obsolete comment: a comment that no longer matches the code is worse than no comment; update or delete it as soon as it drifts.
• C3 - Redundant comment: a comment that just restates what the code already says (i++; // increment i) adds nothing and is one more thing to keep in sync.
• C4 - Poorly written comment: if a comment is worth writing, write it well - correct grammar, no rambling, no stating the obvious.
• C5 - Commented-out code: delete it. Nobody knows how old it is or whether it still matters, and version control already remembers it.

Environment

• E1 - Build requires more than one step: checking out and building a project should be one command each, not a ritual of scripts and manual steps.
• E2 - Tests require more than one step: running the whole test suite should be a single click or command, so nothing discourages you from running it constantly.

Functions

• F1 - Too many arguments: zero is best, then one, two, three; more than three needs a strong justification.
• F2 - Output arguments: readers expect arguments to be inputs; if a function must change something, have it change the state of the object it's called on instead.
• F3 - Flag arguments: a boolean parameter announces that the function does two things disguised as one - split it into two functions instead.
• F4 - Dead function: a method nobody calls should be deleted, not preserved "just in case."

General

• G1 - Multiple languages in one source file: minimize how many languages (SQL, HTML, XML, embedded scripts) live in a single file; each one is a context switch for the reader.
• G2 - Obvious behavior is unimplemented: a function should do what its name leads a reasonable reader to expect, including the parts nobody explicitly asked for.
• G3 - Incorrect behavior at the boundaries: don't trust intuition for corner cases; find every boundary condition and write a test for it.
• G4 - Overridden safeties: don't disable warnings, tests, or checks just to unblock a build - that's how the checks that would have caught the next bug get skipped too.
• G5 - Duplication: the most important smell in the list. Every repeated clump of code, every repeated conditional chain, is a missed abstraction - extract it.
• G6 - Code at wrong level of abstraction: keep high-level policy and low-level detail in separate classes/files; a base class should know nothing about implementation specifics.
• G7 - Base classes depending on their derivatives: a base class that mentions its subclasses by name has the dependency backwards.
• G8 - Too much information: a well-designed interface exposes as little as possible; hide data, utility functions, and constants that callers don't need.
• G9 - Dead code: unreachable branches, unused functions, impossible catch blocks - if it never runs, delete it.
• G10 - Vertical separation: declare variables and private functions close to where they're used, not hundreds of lines away.
• G11 - Inconsistency: if you name or structure one thing a certain way, do all similar things the same way - consistency is a big readability win for almost no cost.
• G12 - Clutter: empty default constructors, unused variables, comments that say nothing
  • remove anything that exists but adds no value.
• G13 - Artificial coupling: don't declare a general-purpose constant or enum inside a specific class just because it was convenient; it forces unrelated code to depend on that class.
• G14 - Feature envy: a method that spends its time calling another object's getters to manipulate that object's data probably belongs on that other object instead.
• G15 - Selector arguments: a trailing boolean or enum that selects behavior is a sign the function should be split into two clearly named functions.
• G16 - Obscured intent: dense one-liners, Hungarian notation, and magic numbers all hide what the code means - favor explicit, expressive code over compact code.
• G17 - Misplaced responsibility: put code where a reader would naturally look for it, not wherever was easiest to write it.
• G18 - Inappropriate static: if there's any chance a function should behave polymorphically later, make it an instance method now, not a static one.
• G19 - Use explanatory variables: break a dense calculation into named intermediate variables; more of them is usually better than fewer.
• G20 - Function names should say what they do: if you have to read the implementation to know what a function does, the name has failed.
• G21 - Understand the algorithm: passing your test cases isn't the same as knowing the algorithm is correct - refactor until the logic is obviously right, not just empirically so.
• G22 - Make logical dependencies physical: if module A assumes something about module B (like a page size), have A ask B for that value explicitly instead of hardcoding the assumption.
• G23 - Prefer polymorphism to if/else or switch/case: switch statements that select behavior by type are usually a sign that subclasses and polymorphism would fit better; keep at most one such switch per type of selection.
• G24 - Follow standard conventions: agree on a team coding standard and let the existing code demonstrate it, rather than writing it down and hoping people read it.
• G25 - Replace magic numbers with named constants: hide raw numeric literals (and any other unexplained literal) behind a name that says what they mean.
• G26 - Be precise: don't be vague about nulls, concurrency, or types "close enough" to what you need - imprecision here is either laziness or an unresolved disagreement.
• G27 - Structure over convention: enforce a design decision with a structure that makes violations impossible (abstract methods) rather than a convention that only discipline enforces (parallel switch statements).
• G28 - Encapsulate conditionals: extract a boolean expression into a well-named function (shouldBeDeleted(timer)) instead of inlining the raw logic at the call site.
• G29 - Avoid negative conditionals: phrase conditions as positives where you can; negatives take an extra mental step to parse.
• G30 - Functions should do one thing: a function with multiple sequential sections doing different jobs should be split into one function per job.
• G31 - Hidden temporal couplings: if calling functions in the wrong order breaks things, make the ordering explicit - have each step's output feed the next step's input.
• G32 - Don't be arbitrary: have a reason for how you structure code, and make that reason visible; an arbitrary structure invites other people to "fix" it into something worse.
• G33 - Encapsulate boundary conditions: don't scatter +1/-1 boundary arithmetic through the code; compute it once into a well-named variable.
• G34 - Functions should descend only one level of abstraction: every statement in a function should sit one level below the function's own name; don't mix "what" with "how."
• G35 - Keep configurable data at high levels: default values and configuration constants belong near the top of the call stack, passed down as arguments, not buried in low-level functions.
• G36 - Avoid transitive navigation: don't chain through collaborators (a.getB().getC()); ask your immediate collaborator to do the work instead ("write shy code," a.k.a. the Law of Demeter).

Java

• J1 - Avoid long import lists by using wildcards: import a whole package once you use two or more classes from it, rather than piling up specific imports.
• J2 - Don't inherit constants: don't gain access to constants by inheriting the interface that declares them; use a static import instead.
• J3 - Constants versus enums: prefer enums to public static final int groups - an enum can carry its own methods and fields, and its meaning can't get lost the way a raw int can.

Names

• N1 - Choose descriptive names: names are most of what makes software readable - don't rush picking them, and revisit them as the code's meaning shifts.
• N2 - Choose names at the appropriate level of abstraction: don't leak implementation details into a name (getConnectedPhoneNumber) when the concept is more general (getConnectedLocator).
• N3 - Use standard nomenclature where possible: name things the way the pattern, the language, or the team's ubiquitous language already does (toString, ...Decorator).
• N4 - Unambiguous names: a name should make a function's actual behavior clear, not just gesture vaguely at its purpose.
• N5 - Use long names for long scopes: i is fine for a five-line loop; a name visible across a whole class needs to carry more information.
• N6 - Avoid encodings: drop scope/type prefixes like m_ or f - today's tools already tell you that information.
• N7 - Names should describe side effects: if a getter also creates the thing it returns, its name should say so (createOrReturnOos, not getOos).

Tests

• T1 - Insufficient tests: a suite is insufficient as long as any condition or calculation goes unexercised - "seems like enough" isn't a metric.
• T2 - Use a coverage tool: coverage reports are the fastest way to find untested branches and dead code.
• T3 - Don't skip trivial tests: they're cheap to write and their documentation value outweighs the cost.
• T4 - An ignored test is a question about an ambiguity: a @Ignored or commented-out test is really an open question about the spec - track it as one.
• T5 - Test boundary conditions: the middle of an algorithm is usually right; the edges are where bugs hide.
• T6 - Exhaustively test near bugs: bugs congregate - once you find one in a function, test that function thoroughly, not just the one failing case.
• T7 - Patterns of failure are revealing: a pattern in which tests fail (all inputs over N characters, all negative second arguments) can point straight at the root cause.
• T8 - Test coverage patterns can be revealing: what passing tests do and don't execute can explain why the failing ones fail.
• T9 - Tests should be fast: a slow test is a test that eventually gets skipped or deleted when time is tight - keep the whole suite fast enough to run constantly.