Chapter 17 Flashcards — Smells and Heuristics
flashcards clean-code code-smells
What does C5 (Commented-Out Code) say and why is it so dangerous?
?
C5 says: delete commented-out code immediately. It festers because other developers see it and assume it must be there for a reason — so nobody removes it. It accumulates, references outdated APIs, and misleads readers. Source control remembers every deleted line; you never need a code comment as a backup.
What is C3 (Redundant Comment) and what is its canonical example?
?
C3 — Redundant Comment: a comment that describes exactly what the code already says, adding no information. The canonical example is i++; // increment i. Redundant comments increase maintenance burden (both code and comment must be updated) and train readers to skip all comments, including valuable ones.
What is the rule C1 sets for what information belongs in a comment?
?
C1 — Inappropriate Information: comments should contain only what a reader needs that cannot live elsewhere. Change logs, author names, ticket numbers, and edit histories belong in source control metadata, not in source files. A comment stuffed with metadata ages badly and is almost never read by anyone who needs it.
What is the ONE SWITCH rule (G23)?
?
G23 — Prefer Polymorphism to If/Else or Switch/Case: there should be at most one switch statement for any given type distinction in the codebase. Every additional switch is a duplication point — adding a new type requires finding and updating every switch. Replace type-based dispatch with polymorphism: define the behavior as a method on each class and let the runtime dispatch automatically.
What is G5 (Duplication) and why does Martin consider it the most important smell?
?
G5 — Duplication (DRY): every duplicated piece of code represents a missed abstraction. Duplication means a future change must be made in N places; at least one will be missed, causing inconsistency. Martin considers it the most important smell because almost every other smell is either caused by duplication or enables future duplication. Some forms are subtle: duplicate switch/if-else chains testing the same type are a sign that polymorphism should replace the conditionals.
What does G4 (Overridden Safeties) describe? Give two examples.
?
G4 — Overridden Safeties: disabling safety mechanisms to make code compile or tests pass. Examples: adding @Ignore to a failing test instead of fixing it; using @SuppressWarnings("unchecked") to silence a real type-safety warning; setting failOnError = false in a build script. Each creates an invisible gap between what you believe the code does and what it actually does. Fix the underlying problem — never silence the signal.
What is G14 (Feature Envy) and what is the fix?
?
G14 — Feature Envy: a method uses the data or methods of another class more than it uses its own class’s data. The method “envies” the other class — it wants to live there. The fix is to move the method to the class whose data it manipulates. After the move, the method has direct access to the data it needs, and the original class becomes cleaner.
What is G28 (Encapsulate Conditionals) and how does it improve readability?
?
G28 — Encapsulate Conditionals: extract complex boolean expressions into a well-named private method. Instead of if (timer.hasExpired() && !timer.isRecurrent()), write if (shouldBeDeleted(timer)). The extracted method reads as prose, hides the implementation details of the condition, makes the condition independently testable, and makes the if statement’s intent obvious without reading the details.
What does G36 (Avoid Transitive Navigation) say? What principle does it formalize?
?
G36 — Avoid Transitive Navigation formalizes the Law of Demeter: a module should talk only to its immediate collaborators, not to the collaborators’ collaborators. The code smell is chains like a.getB().getC().doSomething(). These chains tightly couple the caller to the internal structure of A, B, and C. Any change in the chain forces changes at the call site. Fix: have each object expose what its direct callers need — user.getCityName() instead of user.getAddress().getCity().getName().
What is G31 (Hidden Temporal Couplings) and how do you make the coupling visible?
?
G31 — Hidden Temporal Couplings: when functions must be called in a specific order, that order is enforced only by convention or documentation — not by the code. The fix: make each function return a result that is required as input by the next function. This encodes the order in the type system — you cannot call step 2 without having the output of step 1. Example: MudResult mud = diveForMud(); MudPies pies = makeMudPies(mud); cleanUp(pies);
What is G22 (Make Logical Dependencies Physical)?
?
G22 — Make Logical Dependencies Physical: if one module depends on another having been called first, or assumes some precondition about another module’s state, that dependency is logical but invisible to the compiler. Make it physical: pass the required precondition as a parameter, or structure the return values to encode the required calling sequence. Logical dependencies cause mysterious runtime failures; physical dependencies cause immediate compile errors.
What is G7 (Base Classes Depending on Their Derivatives)?
?
G7: a base class should know nothing about its subclasses. If a base class imports, references, or instanceof-checks a subclass, the dependency arrow has inverted: the abstraction depends on the concretion. This makes it impossible to deploy the base class independently and violates the Dependency Inversion Principle. Use polymorphism — the base class defines the interface; each subclass provides the behavior.
What does G35 (Keep Configurable Data at High Levels) prescribe?
?
G35: constants and configuration values — timeouts, retry counts, feature flags, limits, default values — should live at the highest level of the system and be passed down as parameters to low-level implementations. Burying a constant like int retryCount = 3 inside a low-level utility class makes it invisible and hard to tune per environment. Configuration at the top makes the system’s tunable parameters obvious and easy to change.
What does G19 (Use Explanatory Variables) recommend and why does it cost nothing?
?
G19 — Use Explanatory Variables: break complex expressions — long boolean conditions, chained method calls, dense arithmetic — into intermediate variables with meaningful names. boolean isEligible = isValidEmail && isActiveAccount && hasStrongPassword; makes the condition readable without changing its behavior. It costs zero runtime performance (the compiler optimizes away the variable) and permanently reduces the cognitive load on every future reader.
What is G26 (Be Precise) and what are two concrete ways imprecision manifests?
?
G26 — Be Precise: every decision in code should be explicit and defensible. Imprecision manifests as: (1) returning null when the caller might mishandle it, instead of returning Optional<T> or throwing a named exception; (2) using floating-point equality (==) without an epsilon when approximate comparison is appropriate. Vagueness at one level compounds into bugs at the next. The fix: make every assumption, precondition, and failure mode explicit in the code itself.
What is F3 (Flag Arguments) and why does it violate Single Responsibility?
?
F3 — Flag Arguments: a boolean parameter used to select different behavior paths inside a function is a flag argument. It declares that the function does two things — one for true and one for false. This directly violates Single Responsibility. The fix: split the function into two clearly named functions, one for each behavior. Call sites become self-documenting: renderForSuite() vs renderForSingleTest() instead of render(true).
What is F2 (Output Arguments) and what is the correct alternative?
?
F2 — Output Arguments: using a function parameter as an output — passing in an object and mutating it rather than returning a value — violates the intuition that arguments are inputs. Callers must read the function signature carefully to know that the argument is mutated. The correct alternative: use a return value for output. If the function mutates the state of an object, make it an instance method on that object so the mutation is explicit at the call site.
What is F1 (Too Many Arguments) and what are the ideal argument counts?
?
F1 — Too Many Arguments: the ideal argument count is zero (niladic). One (monadic) is good. Two (dyadic) is acceptable. Three (triadic) should be avoided. Four or more requires special justification and almost never has it. Many arguments signal that the function is doing too much, or that arguments should be collected into a parameter object. Example: makeCircle(double x, double y, double radius, Color color) → makeCircle(Point center, double radius, Color color).
What is G27 (Structure over Convention)?
?
G27 — Structure over Convention: when you have a choice between “we follow a convention” and “the compiler enforces it structurally”, always choose structure. An abstract method that forces every subclass to implement it is more reliable than a convention that says “subclasses should override getTitle().” Conventions can be broken silently; structure violations are caught at compile time. Use abstract methods, type parameters, and interfaces to make correctness automatic.
What is G23’s “prefer polymorphism” rule? When is a switch statement acceptable?
?
G23: replace type-dispatch switch/if-else chains with polymorphism — define the behavior as a method on each class and let the runtime dispatch. A switch statement is acceptable once per type: the factory that creates the right subclass. Every subsequent switch on the same type is a duplication point. The goal is the ONE SWITCH rule: at most one switch per type distinction in the entire codebase.
What is G9 (Dead Code) and how is it different from C5 (Commented-Out Code)?
?
G9 — Dead Code: code that is never executed — unreachable branches after a return, catch blocks for exceptions that can never be thrown, utility functions that no caller ever calls. It differs from C5 (Commented-Out Code) in that it still compiles and runs through CI — it just never executes at runtime. Both should be deleted. Dead code rots silently: the APIs it depends on change, and since nobody runs it, the breakage accumulates invisibly.
What is G11 (Inconsistency) and why does it matter?
?
G11 — Inconsistency: if you name a concept or apply a pattern in one place, apply it identically everywhere similar. If one service stores its collaborator as controller, all equivalent services should use controller — not handler or processor for the same role. Inconsistency forces readers to interpret meaning on every read rather than relying on predictable patterns. Consistency makes a codebase feel like one author wrote it and makes navigation fast.
What do T1 (Insufficient Tests) and T2 (Use a Coverage Tool) say together?
?
T1 says: test everything that could break — if there is no test for something, it will eventually break undetected. T2 says: use a coverage tool to make the gaps in T1 visible. Line coverage highlights which branches have never been exercised. Together, T1 and T2 prescribe: run the tool, look at the red lines, and write tests until they are green. A minimum of 80% line coverage is a useful floor, but 100% branch coverage for critical business logic is the real goal.
What is T5 (Test Boundary Conditions) and what are the 8 boundaries to always test?
?
T5 — Test Boundary Conditions: the happy path is the easiest case to get right; bugs hide at the edges. The 8 boundaries to test for any function: (1) empty input, (2) single element, (3) maximum value/size, (4) minimum value, (5) null, (6) zero, (7) negative numbers, (8) off-by-one at every conditional threshold. Never rely on intuition that boundary cases work — test each one explicitly.
What does T6 (Exhaustively Test Near Bugs) say about where bugs live?
?
T6 — Exhaustively Test Near Bugs: bugs congregate. When you find one bug in a function or module, treat it as a signal that nearby logic also has bugs. After fixing the reported bug, exhaustively test all edge cases, boundary conditions, and paths in the affected area — not just the specific case that was reported. A single failing test often reveals a cluster of latent defects in the same region of code.
What is T9 (Tests Should Be Fast) and why does test speed matter so much?
?
T9 — Tests Should Be Fast: a test that takes seconds to run is a test that will not be run often. If the full suite takes minutes, developers run it only before commits — and the feedback loop becomes hours instead of seconds. Fast tests (< 5ms each) enable TDD: write a test, run it instantly, see it fail, write the code, run it instantly, see it pass. Replace real I/O, real databases, and real networks with mocks and in-memory fakes to achieve fast tests.
What is T4 (An Ignored Test Is a Question) and what should you do with an @Ignore?
?
T4 — An Ignored Test Is a Question: @Ignore or @Disabled on a test means either (a) the feature is not yet implemented, or (b) there is ambiguity about the correct behavior. In neither case is “leave it ignored forever” the right answer. Resolve the ambiguity: define the correct behavior, implement the feature, or delete the test if it is no longer relevant. An ignored test left indefinitely is technical debt that grows because nobody ever feels responsible for resolving it.
What is N7 (Names Should Describe Side-Effects) and what is the canonical example?
?
N7: if a function has side effects beyond what its name implies, the name must describe those effects. The canonical example: a method called getOos() that creates a new ObjectOutputStream if one doesn’t exist is misnamed — get* implies a simple accessor. The correct name is createOrReturnOos(), which honestly declares that the method may create an object as a side effect. Misleading names cause callers to misuse functions and create subtle resource and state bugs.
What is N2 (Choose Names at the Appropriate Level of Abstraction)?
?
N2: names should reflect the abstraction layer they belong to, not an implementation detail. Modem.dial(String phoneNumber) embeds a concrete detail (phone numbers, dialing) into an interface that should be abstraction-level. The correct name is connect(String connectionLocator) — valid whether the modem uses a phone number, an IP address, or any other locator. If an implementation detail leaks into a name, a future change to the implementation forces a name change too.
What is J3 (Constants versus Enums) and what are the three advantages of enums over int constants?
?
J3: use enum instead of public static final int for named constants. The three advantages of enums: (1) type safety — the compiler rejects an integer where an enum is expected; move(42) cannot compile if move(Direction d) requires a Direction; (2) expressiveness — enums can carry methods, fields, and values() iteration; (3) switch exhaustiveness — the compiler (or IDE) warns when a switch on an enum is missing a case. There is no benefit of int constants that enums do not also provide.
What is J2 (Don’t Inherit Constants) and what is the correct alternative?
?
J2: implementing an interface solely to inherit its constants is a misuse of inheritance. It creates a spurious “is-a” relationship and pollutes the implementing class’s public API — all the constants become part of its interface. The correct alternative is static import: import static com.example.HttpConstants.STATUS_OK;. This provides access to the constants without any inheritance relationship.
What is G34 (Functions Should Descend Only One Level of Abstraction)?
?
G34: all statements inside a function should be at the same level of abstraction — exactly one level below the function’s name. A function named createReport() should call fetchReportData() and renderAsHtml(rows) — not execute raw SQL and manually build HTML strings. Mixing levels of abstraction in the same function makes both the high-level intent and the low-level details harder to understand. Extract low-level steps into helper functions.
What is G10 (Vertical Separation) and what is the rule for variable declarations?
?
G10 — Vertical Separation: variables and functions should be defined close to where they are used. Local variables should be declared just above their first use — not at the top of a 60-line function. Private helper functions should appear just below the function that first calls them, not 200 lines away. Long vertical distances force readers to scroll and hold context in working memory. The principle: minimize the vertical distance between a definition and every reference to it.
What is G16 (Obscured Intent) and how does it differ from just having a bad name?
?
G16 — Obscured Intent is broader than a bad name. It covers any code that hides its meaning: abbreviated variable names (m_ldayOfYear), dense one-liners that pack multiple operations together, bit-twiddling tricks, and cryptic formulas. Bad names are one form of obscured intent; the smell also covers structural obscurity. The fix: give everything a clear name, put one operation per line, and save the tricks for proven performance hot paths. Code is written once and read hundreds of times — optimize for the reader.
What is E4 (Dependency on External Resources) and what makes a test fail this smell?
?
E4 — Dependency on External Resources: tests that call real networks, real databases, or live third-party APIs are not repeatable. They fail when the network is slow, the external service is down, the test data has changed, or the environment has no internet access. A test suite must be hermetic — able to run identically with no external dependencies. Fix: mock or stub all external collaborators; replace real databases with in-memory alternatives; pin test data.
What does G33 (Encapsulate Boundary Conditions) solve that G25 (Replace Magic Numbers) does not?
?
G25 addresses magic values (86400 → SECONDS_PER_DAY). G33 addresses magic calculations that recur at code boundaries — level + 1, index - 1, size - 1. These expressions scatter throughout code that traverses trees or arrays. When the boundary logic changes, every instance must be updated and one will be missed. G33’s fix: calculate the boundary once and name it — int nextLevel = level + 1 — then use the named variable everywhere. A single point of change, named for intent.
Total Cards: 35
Review Time: ~25 minutes
Priority: HIGH
Last Updated: 2026-04-14