What a Code Smell Is
A named pattern that raises a question about the design. Roughly half the time the honest answer is "this is fine", and a smell that cannot say when is just taste with a job title.
The requirement, the obvious build, and why it breaks
Every lesson starts where the work starts: someone asked for something, and the first implementation that comes to mind survives until the requirement changes.
When a piece of code looks wrong, how do I tell whether it actually is wrong?
A new engineer opens a pull request rewriting a 90-line function into eleven small ones, because "long functions are a smell". Nothing about the behaviour was asked to change.
Learn the catalogue. Long function, large class, long parameter list, feature envy, data clump — when you see one, fix it. That is what the catalogue is for, and it saves arguing from opinion.
The catalogue was written as a list of *things to look at*, and gets read as a list of things to fix. Those two readings produce opposite pull requests from the same observation.
- The catalogue was written as a list of *things to look at*, and gets read as a list of things to fix. Those two readings produce opposite pull requests from the same observation.
- Applied as rules, smells generate work uncorrelated with change cost: the 90-line parser that has been stable for four years gets rewritten, and the 30-line pricing function copied into six modules does not.
- A rule cannot be argued with, so review turns into citation. "That is a long function" ends a conversation that should have started with "what change does its length make expensive?"
- Once a team fixes smells by count, the count becomes the goal. The eleven small functions satisfy the rule and are harder to read, because the sequence they used to express is now spread across eleven names and a call graph (Long Functions).
What limits the solution, and what must never stop being true
This domain leads with these two. A design that ignores its constraints is not a design, and an invariant nobody named is one nothing is protecting.
- The function is in the payments path and has no characterization tests, so any restructuring is unverified.
- The team has one week of slack this quarter and three committed features.
- Reviewers disagree with each other about the rewrite, which means the review will cost more than the change.
- A refactor must not change observable behaviour; if it might, it is not a refactor and must not be reviewed as one (What Refactoring Actually Is).
- Any claim that code is a problem must name the change it makes expensive, or it is not a claim about the code.
Who owns what, and where the seams fall
Responsibilities decide boundaries; boundaries decide what an interface has to say.
- The smell owns raising the question — nothing more. It is an index into the design questions in this domain, not an answer to one.
- The engineer owns the answer, and the answer must be in terms of a change: what does the next requirement in this area cost as things stand?
- The review owns deciding whether the answer justifies the risk of touching working code now (Review as Design Feedback — and Why It Arrives Too Late).
- The line is between description and prescription. "This class has six reasons to change" is an observation anyone can verify; "this class is too big" is a preference.
- The other line is between smells that point at *knowledge placement* — duplicate knowledge, shotgun surgery, divergent change — and smells that point at *local ergonomics* — long parameter lists, deep nesting. The first group predicts change cost; the second mostly predicts reading effort.
The device, and why `fine` is not a formality
Every smell in this module is written as four things: what you see, what it suggests, when it is genuinely fine, and what the fix would be. The third one is load-bearing. A smell whose author cannot construct a case where the code is right has not described a smell — they have described a rule, and rules about code shape are almost always wrong somewhere.
Here is the module's own subject, written in its own device. Note that the fine case is not a grudging exception: it is the majority case for this particular smell, which is what makes the smell so easy to misuse.
- Looks like — the observable, which anyone can verify without agreeing with you.
- Suggests — the design question it opens, phrased as a question and not a diagnosis.
- Fine when — the case where this exact code is correct. Required, and usually the most useful line.
- Fix — what you would do *if* the question resolves against the code, which is not the same as what you should do now.
looks like Pull requests whose entire justification is a catalogue name. "Extracted method — the function was long." "Introduced parameter object — five arguments." No requirement is mentioned, and the diff touches code nobody has edited in months.
suggests The team has adopted the vocabulary without the second step. The shapes are being read as defects rather than as prompts, so cleanup effort is spread evenly over a codebase where change is not spread evenly at all.
fix Add one required sentence to any cleanup proposal: which upcoming change does this make cheaper? If the sentence cannot be written, close the pull request and leave the code alone. If it can, the review is now about whether that change is likely, which is a question the team can actually answer.
Two families, and only one of them compounds
The catalogue mixes two quite different kinds of observation, and separating them is the single most useful thing you can do with it. Some smells describe where knowledge lives — the same rule in nine modules, one module that changes for seven reasons, a change that reliably touches four packages. Those predict the cost of the next change, and they compound, because each new caller makes the next edit larger.
The others describe local ergonomics: a long parameter list, deep nesting, a badly named variable. They predict reading effort, which is real but bounded and does not compound. Nobody's tenth feature is expensive because a function took six arguments.
This is not a claim that the second family is unimportant. It is a claim about ordering: when you have one afternoon, spend it on the first family, because that is where the arithmetic is.
| Smell | Family | What it predicts | Typical honest answer |
|---|---|---|---|
| Duplicate Knowledge | Knowledge placement | Number of edits per rule change, and the discovery cost of finding them all | Often a real finding — but only once you have checked the copies encode the *same* rule |
| Shotgun Surgery | Knowledge placement | How many modules one requirement moves | A real finding; the boundary is in the wrong place |
| Divergent Change | Knowledge placement | How many unrelated reasons one module has to change | A real finding, though sometimes the module is a deliberate coordination point |
| God Object | Knowledge placement | Contention, regression radius, and reasons to redeploy | Usually real, but the fix is rarely "split it into six classes" |
| Feature Envy | Mixed | Whether behaviour sits next to the data it needs | Genuinely fine surprisingly often — see the lesson |
| Primitive Obsession | Mixed | Whether the type system can catch a class of mistake | Depends almost entirely on what the language makes cheap |
| Long Parameter List | Local ergonomics | Call-site readability and argument-order mistakes | Frequently a symptom of a missing concept, not a missing struct |
| The Utility Dumping Ground | Knowledge placement | Whether anything has an owner | Real, and the fix is ownership rather than tidying |
What to do when you smell something
The sequence below is deliberately short, because a long process would not survive contact with code review. The whole thing takes a couple of minutes and its main output, most of the time, is a decision to do nothing — recorded, so the next reader does not repeat the analysis.
Is this worth changing, now, by you?
when You are implementing a feature here this week and this shape is in the way.
cost Fix it as part of the change, in a separate commit so the review can separate behaviour from structure (Review Size). Cheapest possible moment; take it.
when The pricing rule is in six places and you are working on search.
cost Write it down where the team decides work, with the change it makes expensive. Not a comment in the code — comments about structure decay into folklore (Documentation Decay).
when A gnarly but untouched parser, an ugly but isolated legacy adapter.
cost Do nothing, and say out loud that you looked. Stability is evidence. The cost of touching it is real and the benefit is hypothetical (When Design Does Not Pay).
when Everyone dreads this module and nobody can say why in one sentence.
cost This is the interesting case, and the answer is investigation rather than cleanup: map what the module knows and every reason it has to change before proposing anything (Designing by Responsibility).
when Style, naming, nesting, in a module the suite would not catch a regression in.
cost Leave it. The expected value is negative: unverifiable risk against an aesthetic gain (Refactoring Without Tests).
How to build it
Most important first.
- Treat every smell as a question with a required second step: name the requirement change that this shape makes expensive. If you cannot name one, you have found a preference, and preferences do not justify touching production code.
- Ask whether the code has actually changed. Version history is the cheapest evidence available: a smelly module nobody has edited in three years is costing you nothing (The Cost of Change).
- Prefer smells that are about where knowledge lives, because those are the ones that compound (Duplicate Knowledge, Shotgun Surgery).
- When the answer is "this is fine", say so explicitly rather than silently. An unrecorded "we looked and it was fine" gets re-litigated by the next reader every six months.
- Fix opportunistically, inside changes you were making anyway, which is where the cost is lowest and the tests are already being run (The Refactoring Loop).
What the next change costs
The field this whole domain exists for. A structure is only better if it makes the change after this one cheaper — and it is worth saying which changes it does not help.
- Under the rule-based reading, the next change costs whatever it cost before, plus the review time spent on shapes unrelated to it, plus the regression risk of restructuring that was never needed.
- Under the question-based reading, the next change costs the same edit plus a few minutes of asking whether this area is where knowledge went missing — and occasionally a targeted fix that makes the change after it cheaper.
- The measurable difference is not code quality. It is that cleanup work becomes correlated with where changes actually arrive, and stops being distributed evenly over a codebase where change is not.
- Requiring a named change before cleanup slows down engineers whose instincts are good, and some of that friction is pure loss.
- Treating smells as questions makes them harder to enforce, and harder to enforce means less consistent across a large team. Rules scale; judgement does not.
- The honest version takes longer in review. "Why is that a problem here?" is a slower conversation than "the linter says so", and it is a conversation you have to be willing to keep having.
What can go wrong
- Smell-driven refactoring: continuous restructuring that produces churn, review load and regression risk, and no measurable improvement to the cost of the next feature.
- Smell blindness: the catalogue trains you to see the listed shapes, so unlisted problems — a hidden temporal dependency, an invariant enforced nowhere — stop registering because they have no name.
- The mitigation fails too: demanding a named change before any cleanup lets genuine rot accumulate behind "nobody has asked for anything here yet", right up until someone does.
- Every smell judgement depends on the expected change profile of the area, which is domain knowledge, not code knowledge.
- Acting on a smell depends on a test suite that would notice a behaviour change; without one the fix is a rewrite wearing a refactor's name (Refactoring Without Tests).
- "So smells do not matter." They matter a great deal — as attention direction. The catalogue is a trained eye, distributed. What it is not is a defect list.
- "If there is no smell, the design is fine." Most expensive designs smell of nothing at all. A rule duplicated in six well-named, well-formatted, individually reasonable modules has no local smell whatsoever (What Makes Software Hard to Change).
- "Half the time it is fine means it is a coin flip." It means the shape is evidence, not proof. Evidence that is right half the time is extremely useful if the check is cheap, which is exactly the situation here.
- "A smell is technical debt." It is not. Debt is a choice that raises the cost of future change; a smell is a pattern that suggests such a choice may have been made (What Technical Debt Actually Is).
- code-smells
Testing it, and how it ages
- Before acting on any smell, check that a test would fail if the behaviour changed. That check is itself the most useful thing the smell produced (Characterization Tests).
- Test at the boundary you intend to keep, not at the internals you are about to move, or your tests become the reason the cleanup stalls (What a Unit Is).
- For smells about knowledge placement, the useful test is a thought experiment: change the rule, and see how many test files you had to open.
- A team's smell list should shrink and specialise over time. The generic catalogue is a starting point; after two years you should know which three shapes have actually predicted pain in *this* codebase.
- Some smells stop applying entirely when the language changes underneath you: primitive obsession is a different argument in a language with cheap newtypes than in one without (Primitive Obsession).
- Static analysis will keep offering to count these for you. Counting is fine; publishing the count as a measure of quality is where it goes wrong, because the count has no idea which module the next requirement lands in.
Where this applies
This domain's advice is contested more than most. These labels say what each claim is specific to — and where CONTESTED appears, the note gives the strongest form of the opposing view rather than a caricature.
- GENERALThat a recurring shape can indicate a design problem without proving one holds in every language and paradigm; what differs is which shapes recur, since a functional codebase produces different ones than an OO codebase.
- CONTESTEDThe strongest opposing position is that smells are worse than useless because they license restructuring by taste: the catalogue provides vocabulary that makes an aesthetic preference sound like an engineering finding, and teams that adopt it demonstrably spend review cycles on shapes rather than on behaviour. The defence is that the alternative is not neutrality but *unnamed* taste, which is harder to argue with, not easier.
- SCALE-SPECIFICOn a two-person codebase everyone knows where the knowledge lives, so smells add little over conversation; at fifty engineers the shared vocabulary is most of the value, because it lets a reviewer flag something in a module they do not own without claiming authority over it.
Where the depth lives
This domain teaches the codebase-level structure and hands the rest off.
- — Testing & Reliability Engineering — every judgement in this module assumes you can tell whether a restructuring changed behaviour, which is a coverage question this domain leans on and does not answer.