Code Smell Inspector

A systematic, language-agnostic guide for inspecting code structure quality, readability, and test health. Use this checklist during code review to identify structural problems before they compound.

Source: Generalized from the original PHP/Laravel-focused Smell Inspector, supplemented with community best practices from Martin Fowler’s Refactoring, Robert C. Martin’s Clean Code, and modern software engineering practice.

Severity Levels

Level	Meaning
🔴 MUST FIX	Affects correctness, security, or long-term maintainability. Blocking.
🟡 SUGGEST	Improves the code but doesn’t block merge. Address in a near-term iteration.
🟢 OK	This dimension passes inspection.
💡 NOTE	Worth noting for discussion — not necessarily a problem.

---

Part 1: Structural Code Smells

Structural smells betray problems with class design, method boundaries, and module coupling. They accumulate silently and make future changes progressively harder.

1.1 Long Method / God Function

Trigger: Function exceeds 30 lines; function name contains and/or/also; comments partition the function into logical “paragraphs.”

Why it hurts: Long methods resist change — you can’t modify one behavior without risking another. They’re hard to test because each test must set up state for the entire method.

Remedy: Extract private methods by responsibility. A method should do one thing: either orchestrate or compute, not both.

Language notes:

• In functional languages, long pipelines (|>) are acceptable if each step is a named, testable function.
• In async code, split long async functions at await boundaries — each segment becomes independently testable.

1.2 God Class

Trigger: Class exceeds 200 lines; more than 10 public methods; class name contains Manager/Service/Helper/Utils with mixed responsibilities.

Why it hurts: God classes accumulate unrelated concerns. Adding a feature means touching the same class, creating merge conflicts and regression risk.

Remedy: Split by responsibility boundary. Look for hidden sub-domains — groups of methods that only interact with each other and a subset of fields.

Language notes:

• In Go, prefer small interfaces (1–3 methods) over large structs.
• In Rust, use module-level organization with impl blocks for distinct concerns on the same type.

1.3 Feature Envy

Trigger: A method accesses another object’s fields/methods more than its own; chains of other.getX().getY().getZ().

Why it hurts: The method is in the wrong place. It couples two classes unnecessarily and violates “data and behavior together.”

Remedy: Move the method to the class it envies. If moving is awkward (the method serves two masters), extract the envied data into a new class that owns both the data and the method.

Language notes:

• In OOP languages (Java, C#, PHP, Python, Ruby, TypeScript): classic refactoring target.
• In procedural/functional code, this manifests as a function that takes a large struct/record and only reads one field — consider narrowing the parameter.

1.4 Data Clumps

Trigger: The same 3+ parameters appear together across multiple function signatures; the same fields are always read/written together.

Why it hurts: Data that travels together should live together. Clumps scattered across signatures create shotgun surgery when the shape changes.

Remedy: Extract into a value object, DTO, or struct. The new type can also host validation and formatting logic.

Examples across languages:

// Before: clump
function schedule(start: Date, end: Date, timezone: string): void {}
function reschedule(start: Date, end: Date, timezone: string): void {}

// After: value object
class TimeWindow { start: Date; end: Date; timezone: string }
function schedule(window: TimeWindow): void {}

// Before
fn schedule(start: DateTime, end: DateTime, tz: Tz) {}
fn reschedule(start: DateTime, end: DateTime, tz: Tz) {}

// After
struct TimeWindow { start: DateTime, end: DateTime, tz: Tz }
fn schedule(window: TimeWindow) {}

1.5 Primitive Obsession

Trigger: string literals for state ("active"/"pending"); raw int for IDs, amounts, or durations; bare arrays/dicts for structured data.

Why it hurts: Primitives carry no domain meaning. A string status can hold any value, including typos. An int amount doesn’t carry currency context.

Remedy: Introduce enums, value objects, or strongly-typed DTOs.

Exemptions:

1. If the type system provides exhaustive checking with union types (e.g., TypeScript 'active' | 'pending' | 'archived', PHPStan union types), and there are ≤3 variants, string unions are acceptable — the compiler enforces correctness equivalently to an enum.
2. Values from external systems (API responses, user input, file parsing) are necessarily strings at the boundary. Use DTOs to map at the boundary, then flow typed values internally.

Language notes:

• Rust: enum Status { Active, Pending, Archived } — zero-cost abstraction.
• TypeScript: type Status = 'active' | 'pending' | 'archived' — compile-time safety.
• Go: type Status string; const (StatusActive Status = "active"; ...) — iota for int-based enums.
• Python: from enum import StrEnum (3.11+) or Enum.
• Java: enum Status { ACTIVE, PENDING, ARCHIVED }.

1.6 Long Parameter List

Trigger: Function has more than 4 parameters; multiple boolean flags in the signature.

Why it hurts: Many parameters obscure what the function actually needs. Boolean flags are especially damaging — process(true, false) is completely opaque at the call site.

Remedy: Merge related parameters into an object. Split boolean flags into separate, well-named methods.

// Before
function search(term: string, page: number, size: number, sortBy: string, ascending: boolean): Result {}

// After
function search(term: string, opts: SearchOptions): Result {}
// Or: searchNewest(term), searchOldest(term) — no boolean flag

Language notes:

• Python: keyword-only arguments (*, page: int, size: int) mitigate opacity at call sites.
• Kotlin/Swift: named arguments achieve similar clarity; booleans are still a smell.

1.7 Divergent Change

Trigger: One class is modified for different business reasons (e.g., formatting logic changes when persistence logic changes).

Why it hurts: Violates Single Responsibility Principle. Two unrelated change streams share a file, creating merge conflicts and cognitive overhead.

Remedy: Split the class so each reason for change lives in its own module.

1.8 Shotgun Surgery

Trigger: A single feature change requires modifying 3+ files/classes; changing one thing forces a global search-and-replace.

Why it hurts: The opposite of Divergent Change — one concern is scattered. Forgetting one file causes a bug.

Remedy: Consolidate the scattered logic into a single module. Use the IDE’s “find usages” to verify completeness.

1.9 Switch/Match Proliferation

Trigger: Identical switch/match structures repeated across the codebase; adding a new case requires changes in multiple places.

Why it hurts: Each repetition is a place to forget the new case. Violates the Open/Closed Principle.

Remedy: Replace with polymorphism (Strategy/Command pattern) or a centralized factory.

// Before: proliferation
function calculatePrice(p: Product) {
  switch (p.type) { case 'book': return p.price * 0.9; case 'food': return p.price; }
}
function getShipping(p: Product) {
  switch (p.type) { case 'book': return 5; case 'food': return 10; }
}
// After: polymorphism
interface PricingStrategy { price(p: Product): number; shipping(p: Product): number; }

Language notes:

• Rust: match with exhaustive patterns is the idiomatic approach — proliferation still applies when the same match shape repeats across functions.
• FP languages: use pattern matching in a single function, not scattered across the codebase.

1.10 Refused Bequest

Trigger: Subclass inherits from parent but overrides most methods with throw new UnsupportedOperationException(); subclass uses only a small portion of parent.

Why it hurts: Inheritance implies “is-a.” A subclass that rejects half the interface is not truly a subtype. Callers expecting the parent contract get runtime failures.

Remedy: Replace inheritance with composition. If only a few methods are needed, extract those into a separate interface or trait.

1.11 Middle Man

Trigger: A class/function where >90% of methods are pure delegation to another object, with negligible own logic.

Why it hurts: Adds an indirection layer that doesn’t earn its keep. Every new method on the delegate requires a matching pass-through.

Remedy: Remove the middle man; let callers use the delegate directly. If the middle man exists for future extensibility that never materialized, delete it.

When delegation IS justified: Caching, logging, access control, or API compatibility layers — these add behavior, not just forwarding.

1.12 Tramp Data (Pass-Through Parameter)

Trigger: A parameter passes through ≥2 call levels without being read or transformed by intermediate functions. Intermediate signatures “lie” about their dependencies.

Why it hurts: Creates a false dependency chain. When the leaf function changes its parameter shape, every intermediate signature must update (shotgun surgery enabler). Tests for intermediate functions are forced to construct meaningless placeholder values.

Not a smell when: Dependency injection at construction time; pipeline/middleware frameworks passing a context object by convention.

Remedy:

• If the leaf needs a stable global source (config, singleton), read it directly there.
• If the dependency is variable, inject it once at construction, not per-call.
• For pure functions, split into “prepare data” + “apply data” phases to eliminate pass-through.

1.13 Hidden Side Effects

Trigger: Method named getX()/findX()/fetchX()/loadX() (read semantics) but internally performs writes — database INSERT/UPDATE/DELETE, sending emails, mutating global state, writing files.

Why it hurts: Callers read getUser(id) and assume it’s safe to call in a loop or at any point. When it has side effects, every caller must know implementation details to use it safely.

Not a smell when: Network queries, log writes, cache reads, throwing exceptions, emitting events — these are conventionally accepted as non-mutating in most ecosystems.

Remedy: Extract write operations into explicitly named methods (createUser(), updateUser()). A read method should be callable any number of times without changing system state.

Command-Query Separation (CQS): A method should either be a command that performs an action (and returns nothing) or a query that returns data (and has no side effects) — never both.

1.14 Inappropriate Intimacy

Trigger: Two classes access each other’s private fields/methods; bidirectional dependencies.

Why it hurts: The classes cannot be understood or tested independently. Changes to either class ripple to the other.

Remedy: Introduce an interface to decouple. Extract shared logic into a third class. Use events/observer to break the bidirectional dependency.

1.15 Superset Naming

Trigger: Class/namespace/method name suggests broader capability than the implementation actually provides.

Why it hurts: Misleads readers and callers. PaymentProcessor that only handles credit cards sets wrong expectations. Future developers may add check support in the wrong place because the name invited it.

Remedy: Narrow the name to match the implementation (CreditCardProcessor), or expand the implementation to match the name.

1.16 Sealed Variation Point

Trigger: An abstract class’s template method calls private helper methods, preventing subclasses from overriding that behavior.

Why it hurts: The Template Method pattern explicitly intends certain steps to be variable. Marking them private seals a variation point that should be open.

Remedy: Change to protected if the method is a legitimate variation point. If it genuinely should never vary, document why.

Language notes:

• Java/C#: private ≠ protected — check intentionality.
• Python: no true private — _method convention serves; __method (name mangling) should rarely be used in template methods.
• Go: no inheritance — not applicable, but embedding + interface composition achieves similar patterns.

1.17 Mixed Levels of Abstraction

Trigger: Within a single method, low-level details (concrete filter rules, string manipulation) are called at the same level as high-level abstractions (delegated strategy methods).

Why it hurts: Readers must constantly zoom in and out. The method fails the “newspaper metaphor” — you can’t read it at a consistent level of detail.

// Mixed levels
function processOrders(orders: Order[], strategy: PricingStrategy) {
  const valid = orders.filter(o => o.status !== 'cancelled' && o.total > 0);      // low-level
  const priced = strategy.apply(valid);                                             // high-level
  const formatted = priced.map(p => ({ ...p, display: `$${p.total.toFixed(2)}` })); // low-level
  return formatted;
}

Remedy: Extract low-level details into named functions at the same abstraction level as the strategy call:

function processOrders(orders: Order[], strategy: PricingStrategy) {
  const valid = filterValidOrders(orders);      // same level
  const priced = strategy.apply(valid);          // same level
  return formatForDisplay(priced);               // same level
}

1.18 Flag Variable (Implicit State Machine)

Trigger: A boolean variable tracks “has X happened” and drives branching behavior later in the same function. The flag encodes an implicit state machine (BEFORE/INSIDE/AFTER) whose transitions are scattered through the loop body.

Why it hurts: Readers must reconstruct the full state diagram in their heads to understand control flow. The flag’s meaning at any point depends on execution history, not local context.

Remedy:

• On collections: use declarative chains (skipUntil + takeUntil) instead of flag-driven loops.
• Extract “locate phase” and “consume phase” into separate methods, eliminating inter-phase flag passing.
• If a loop must remain, rename the flag to express the current phase (inTargetSection), not the historical event.

1.19 Contract-Drift Naming

Trigger:

• Return type contract: getX() does not return type X.
• Exception contract: validateX() returns bool when callers expect void (throw on invalid); or checkX() throws when it implies inspection-only.
• Layer boundary leak: showX() in a non-presentation layer — show/render/display belong to the presentation tier.
• Implementation leak: getBySymbol() when lookup strategy changed from symbol to ID; parameterized behavior with a fixed-strategy name.

Why it hurts: The name is the primary API documentation. When it drifts from behavior, every caller must read the implementation to know what actually happens.

Not a smell when: Deprecated methods with @deprecated annotation, documented replacement, and migration timeline.

Remedy: Name for the caller’s contract, not the implementation. When semantics change, create a new method and preserve the old as a compatibility shim.

1.20 External Contract Coupled to Internal Schema

Trigger: Public API response keys reuse internal database column names or model constants; transform/serialization layers lack explicit field mapping, so field names drift with internal refactoring.

Why it hurts: External API consumers break when you rename a database column. The internal schema becomes frozen by external contracts.

Remedy: Maintain an explicit mapping layer between external field names and internal schema. Version your API. Require that internal refactors audit all serialization/transform layers.

1.21 Mixed Type Contract

Trigger: Parameters lack type annotations; types use any/mixed/unknown without narrowing; Map<string, any> proliferates internally.

Why it hurts: The type checker can’t help. A Map<string, any> tells callers nothing about shape, required keys, or value types.

Not a smell when: At external system boundaries (raw API payloads, message queues) — but should be narrowed to a DTO immediately after parsing.

Remedy: Define explicit types. Prefer { name: string; age: number } over Record<string, any>. For generics/traits that depend on host class shape, use constrained generics or explicit host contracts.

1.22 Framework Reimplementation

Trigger: Hand-written logic duplicates a framework built-in — custom string masking when the stdlib has it; manual sort when the language provides it; custom unique-ID dedup when the framework offers it.

Why it hurts: More code to maintain, more surface for bugs. Framework implementations are tested by millions of users; yours is tested by you.

Not a smell when: The framework capability has a documented limitation that matters, with a comment explaining the tradeoff.

Remedy: During review, require a brief justification when a built-in equivalent exists. Prefer framework/standard library capabilities unless there’s a measured reason not to.

1.23 Mutable-Constant Smell

Trigger: An instance field is assigned once (declaration or constructor) and never mutated: no setter, no conditional reassignment throughout the object’s lifecycle. Queue names, cache keys, pattern strings as instance properties.

Why it hurts: Instance fields signal “this can change per object.” Constants signal “this never changes, period.” Using a field for a constant misleads readers and wastes memory per instance.

Remedy: Use const/static final/class-level constant. If subclasses override behavior via field override, use a template method or configuration object instead.

Language notes:

• PHP 8.3+: private const string FOO = 'value' with typed constants.
• TypeScript: static readonly FOO = 'value' or as const.
• Java: private static final String FOO = "value".
• Python: class-level FOO: ClassVar[str] = "value".
• Go: unexported package-level const foo = "value".

1.24 Interface Constructor Lock-in

Trigger: An interface/protocol/trait declares a constructor signature, forcing all implementations to accept the same dependency shape.

Why it hurts: Different implementations need different dependencies. A constructor in an interface prevents the DI container from varying injection per implementation without interface changes.

Remedy: Interfaces should constrain behavior, not construction. Construction dependencies belong to implementations or factory configurations. If factory behavior must be constrained, define a separate FactoryInterface.

---

Part 2: Readability Checklist

2.1 Naming Quality

Rule	Good	Bad
Functions: verb phrase	getUserById, calculateTax	user, tax
Classes: noun	UserRepository, Invoice	UserData, DoStuff
Booleans: is/has/can/should	isActive, hasPermission	active, flag
Collections: plural	users, items	userList, itemArray
Avoid abbreviations	repository, configuration	repo, cfg
No misleading names	getUser should not create a user	—

Common abbreviations that ARE acceptable: id, url, dto, http, sql, json, xml, io, db.

2.2 Cognitive Complexity

Nesting depth: Conditionals/loops nested beyond 3 levels trigger this rule.

Remedy: Early return / guard clauses to flatten the happy path. Extract nested logic into named functions.

// Before: deep nesting
function process(order: Order) {
  if (order.isValid()) {
    if (order.hasItems()) {
      if (order.isInStock()) {
        // actual logic here
      }
    }
  }
}

// After: guard clauses
function process(order: Order) {
  if (!order.isValid()) return;
  if (!order.hasItems()) return;
  if (!order.isInStock()) return;
  // actual logic here
}

Compound conditions: A single if with more than 3 sub-conditions.

Remedy: Extract into a named variable or predicate function:

// Before
if (user.age > 18 && user.country === 'US' && !user.isBlocked && user.hasVerifiedEmail) { ... }

// After
const isEligible = user.age > 18 && user.country === 'US' && !user.isBlocked && user.hasVerifiedEmail;
if (isEligible) { ... }

Else abuse: else blocks that could be eliminated with early return.

// Before
function getLabel(status: Status): string {
  if (status === 'active') {
    return 'Active';
  } else {
    return 'Inactive';
  }
}

// After
function getLabel(status: Status): string {
  if (status === 'active') return 'Active';
  return 'Inactive';
}

2.3 Function Length

Threshold	Action
≤20 lines	Generally fine
20–40 lines	Review — can it be split?
>40 lines	Must be split

These are guidelines, not laws. A 25-line function that’s a straightforward sequence with no branching may be clearer as-is than split into micro-functions.

2.4 Comment Quality

Delete comments that state the obvious:

// Get the user — DELETE THIS
const user = getUser(id);

Keep comments that explain WHY:

// Must use read replica — primary is under load during billing batch
const invoices = readReplica.query('SELECT * FROM invoices');

TODO comments: A // TODO without an associated issue/ticket ID is technical debt without a payment plan. Flag it.

2.5 Opaque External Identifiers

Trigger: External system codes/identifiers (vendor API field names like H1, H4, S3069) appear as variable names, method names, or dictionary keys beyond the parsing boundary.

Why it hurts: Readers cannot understand what H1 means without consulting external documentation. The domain concept is hidden behind an opaque vendor code.

Not a smell when: Inside network-layer request/response DTOs; in test fixtures mocking external responses; directly reading vendor source files — these are faithfully representing the external protocol.

Remedy: Map at the boundary. After parsing, flow domain objects with meaningful names:

// At boundary (acceptable)
interface MorningstarResponse { H1: string; S3069: number; }

// After mapping (internal)
interface FundProfile { fundName: string; expenseRatio: number; }

2.6 Magic Values

Trigger: Bare numeric literals (7, 86400, 0.15); bare string status values ("pending", "active").

Remedy: Named constants or enums.

// Before
if (retries > 3) { ... }
setTimeout(handler, 86400);

// After
const MAX_RETRIES = 3;
const SECONDS_PER_DAY = 86400;
if (retries > MAX_RETRIES) { ... }
setTimeout(handler, SECONDS_PER_DAY);

Exemption: A value used exactly once, with a named parameter or variable that clearly explains its meaning — e.g., retry(times: 3) — is acceptable. Both conditions (single use + clear context) must hold.

---

Part 3: Test Smells

3.1 Test Case Misclassification

Trigger: A test class contains tests unrelated to its stated subject; the same assertion (e.g., “returns 401 for wrong password”) repeated across multiple endpoint test classes.

Why it hurts: Makes it hard to find all tests for a behavior. When the behavior changes, you must find and update scattered copies.

Remedy: Organize test classes by behavior theme, not by endpoint/class under test. Cross-cutting concerns (auth failure, permission denied) belong in dedicated test classes tested once.

3.2 Fragile Test

Trigger: Tests assert private fields or internal intermediate state; tests call internal methods to set up state instead of using the public API; internal refactors (no behavior change) break tests.

Why it hurts: Tests should verify behavior, not implementation. Fragile tests discourage refactoring — developers avoid improving code structure because they don’t want to fix 20 broken tests.

Not a smell when: Verifying side effects not visible through return values — e.g., asserting a domain event was published, a database row was inserted, an email was queued.

Remedy: Test through public APIs. Follow Arrange-Act-Assert: set up via public constructors/factories, act via public methods, assert via public queries.

3.3 Optional-Parameter Laziness

Trigger: A business-critical parameter has a default value, so callers that omit it get silent wrong behavior instead of a compile error.

Why it hurts: The default masks a missing decision. Callers that should be forced to think about the value get a free pass.

Remedy: Remove the default. Make critical parameters required. If a default strategy truly exists, expose it as an explicit named factory method — not a hidden default.

---

Part 4: Supplement — Community Best Practices

The following smells, drawn from Martin Fowler’s Refactoring, Robert C. Martin’s Clean Code, and modern software engineering practice, extend the base checklist above.

4.1 Duplicated Code (DRY Violation)

Trigger: The same (or near-same) block of code appears in multiple places.

Why it hurts: Every copy is a place to forget a fix. Duplication is the root of all evil in software — it multiplies bugs and maintenance cost linearly with copies.

Remedy: Extract into a shared function/method. For near-duplicates, parameterize the difference.

The Rule of Three: Duplication on the first occurrence is fine. On the second, note it. On the third, refactor.

4.2 Dead Code

Trigger: Functions, classes, variables, or branches that are never executed. Unreachable code after return/throw. Imports that are never used.

Why it hurts: Dead code is a distraction. Readers spend time understanding code that has no effect. It may also mask bugs — “why is this here if it’s never called?”

Remedy: Delete it. Version control remembers. Use tree-shaking/linters to detect automatically.

4.3 Mutable Global State

Trigger: Global variables, singletons with mutable state, module-level mutable variables.

Why it hurts: Global state makes tests order-dependent and non-deterministic. Two tests running in parallel may interfere. Reasoning about program behavior requires tracking state across the entire codebase.

Remedy: Pass dependencies explicitly. Use dependency injection. For truly global configuration, use immutable constants.

4.4 Leaky Abstraction

Trigger: An abstraction exposes implementation details that callers must understand to use correctly. SQL errors propagating through a repository interface. Timeout parameters that only make sense for one implementation.

Why it hurts: The abstraction fails its purpose — callers must still understand the underlying implementation to use it safely.

Remedy: Either fix the abstraction to truly encapsulate the detail, or remove the abstraction and let callers use the underlying API directly. A bad abstraction is worse than no abstraction.

4.5 Message Chains (Law of Demeter Violation)

Trigger: a.getB().getC().getD().doSomething(). The caller navigates through multiple objects to reach the method it needs.

Why it hurts: The caller is coupled to the entire intermediate object graph. If any link in the chain changes type, the caller breaks.

Remedy: “Tell, don’t ask.” Have a expose a method that does what the caller wants:

// Before: message chain
const dept = employee.getDepartment();
const manager = dept.getManager();
const name = manager.getName();

// After: tell, don't ask
const name = employee.getManagerName();

4.6 Speculative Generality

Trigger: Abstract classes with one concrete implementation; interfaces that exist “in case we need to swap implementations”; configuration hooks for scenarios that never materialize.

Why it hurts: YAGNI (You Aren’t Gonna Need It). Speculative code must be maintained, tested, and understood — for zero current value.

Remedy: Delete it. Build for today’s requirements. When the second implementation arrives, extract the interface then — with real data about what the abstraction boundary should look like.

4.7 Temporary Field

Trigger: An object field that is only valid during a specific phase of the object’s lifecycle. Callers must know when the field is “ready” to use it safely.

Why it hurts: The object’s state space has invalid configurations that the type system doesn’t prevent. Using the field at the wrong time produces runtime errors.

Remedy: Extract the temporary field and the methods that use it into a separate class whose constructor guarantees the field is populated.

4.8 Data Class (Anemic Model)

Trigger: A class with only fields and getters/setters, no behavior. All logic that operates on the data lives elsewhere.

Why it hurts: Violates “data and behavior together.” The logic is scattered across service classes, and the data class can’t enforce its own invariants.

Remedy: Move relevant behavior into the data class. Ask: “what operations does this data naturally support?” Put those on the class.

4.9 Poltergeist

Trigger: A class that exists only to call another class and then disappears. Short-lived objects with no state that serve purely as intermediaries.

Why it hurts: Adds indirection without value. The call chain is longer than it needs to be.

Remedy: Delete the intermediary. Have the original caller invoke the final target directly.

4.10 Cyclomatic Complexity

Trigger: A function has many branching paths (if/else, switch, loops, try/catch). Tools can measure this: >10 is a warning, >20 is a problem.

Why it hurts: High cyclomatic complexity correlates strongly with bug density. Each branch doubles the number of test cases needed for full coverage.

Remedy: Extract branches into separate functions. Use polymorphism to replace conditional logic. Use lookup tables/maps instead of switch chains.

4.11 Parallel Inheritance Hierarchies

Trigger: Every time you create a subclass of X, you must also create a corresponding subclass of Y. The class hierarchies mirror each other.

Why it hurts: Doubles the number of classes. Adding a variant requires changes in two hierarchies — easy to forget one.

Remedy: Collapse one hierarchy. Have instances of one hierarchy reference instances of the other, rather than subclassing both.

4.12 Inconsistent Error Handling

Trigger: Some functions throw exceptions, others return error codes, others return null, others log-and-swallow — within the same module.

Why it hurts: Callers can’t establish a consistent error-handling pattern. Every call site must read the implementation to know how errors surface.

Remedy: Choose one error-handling strategy per module/layer. Document it. Enforce it in review.

4.13 Lazy Class

Trigger: A class that does too little to justify its existence — a single method, or pure delegation with no added behavior.

Why it hurts: Every class has a cognitive cost. Classes that don’t pull their weight should be inlined.

Remedy: Inline the class into its caller. If it’s a strategy with one implementation, inline until the second implementation arrives.

4.14 Alternative Classes with Different Interfaces

Trigger: Two classes that do the same thing but with different method names/signatures.

Why it hurts: Callers must learn two interfaces for the same concept. Refactoring to unify them later is harder because callers have diverged.

Remedy: Unify to a common interface. Rename methods to match. Use an adapter for the transition period.

4.15 Incomplete Library Class

Trigger: A library/third-party class is missing a method you need, and you’ve scattered the missing behavior across multiple call sites.

Why it hurts: Each call site reimplements the missing behavior slightly differently. Bugs in one fix don’t propagate to others.

Remedy: Use extension methods (C#), monkey-patching (Python — sparingly), wrapper/decorator classes, or submit a PR to the library.

---

Part 5: Language-Specific Considerations

Static vs. Dynamic Typing

• Static languages (TypeScript, Rust, Java, Go, C#, Kotlin, Swift): The compiler catches many smells at build time. Focus review attention on smells the type system can’t express: naming, abstraction levels, side effects, contract drift.
• Dynamic languages (Python, Ruby, JavaScript, PHP without strict types): Type-related smells (Primitive Obsession, Mixed Type Contract, Contract-Drift Naming) are more critical because the runtime won’t catch them. Be stricter about type annotations, docstrings, and runtime checks at module boundaries.

Object-Oriented vs. Functional

• OOP (Java, C#, PHP, Python, Ruby, TypeScript classes): Most structural smells apply directly. God Class, Feature Envy, Refused Bequest, and Inappropriate Intimacy are OOP-centric.
• Functional (Haskell, Elm, Clojure, F#, OCaml, Reason): God Class and Refused Bequest don’t apply. Instead, watch for: large records/tuples (≈ Data Clumps), deeply nested pattern matches (≈ Switch Proliferation), excessive partial application chains (≈ Tramp Data).
• Multi-paradigm (Rust, Scala, TypeScript, Kotlin, Swift): Apply OOP smells to class hierarchies and FP smells to function/module-level code. The boundary is where you switch paradigms.

Memory-Managed vs. Systems Languages

• Managed (Java, C#, Go, Python, JS/TS, Ruby, PHP): GC removes a class of bugs but introduces others — watch for memory leaks via unintended reference retention (caches, event listeners, closures).
• Systems (Rust, C, C++, Zig): Ownership, borrowing, and lifetimes add dimensions to review: unnecessary clones, overly broad locks, unsafe blocks without documentation.

Web/Mobile Framework Notes

• React/Vue/Svelte: Watch for over-large components (God Class equivalent), prop drilling (Tramp Data equivalent), and effects with hidden dependencies.
• Backend frameworks (Express, FastAPI, Rails, Laravel, Django, Spring): Watch for “fat controllers” (God Class), middleware that depends on ordering, and implicit request-scoped state.

---

Review Workflow

1. Start with the diff. Don’t review the entire file — focus on what changed and its immediate context.
2. Scan top-down. Structural smells first (will this change compound?), then readability (can the next person understand this?), then tests (are we confident it works?).
3. One finding per issue. Don’t bundle unrelated problems into one comment. Each finding gets its own analysis.
4. Suggest, don’t command. “Consider extracting a validateOrder method” not “Extract this into a method.” The author knows context you don’t.
5. Severity calibrates urgency. Not every finding is a blocker. Use the severity levels to signal whether this must be fixed now or can wait.

---

Summary Table

Category	Smell	Severity Default
Structural	Long Method / God Function	🟡
Structural	God Class	🔴
Structural	Feature Envy	🟡
Structural	Data Clumps	🟡
Structural	Primitive Obsession	🟡
Structural	Long Parameter List	🟡
Structural	Divergent Change	🔴
Structural	Shotgun Surgery	🔴
Structural	Switch/Match Proliferation	🟡
Structural	Refused Bequest	🔴
Structural	Middle Man	🟡
Structural	Tramp Data	🟡
Structural	Hidden Side Effects	🔴
Structural	Inappropriate Intimacy	🔴
Structural	Superset Naming	🟡
Structural	Sealed Variation Point	🟡
Structural	Mixed Levels of Abstraction	🟡
Structural	Flag Variable	🟡
Structural	Contract-Drift Naming	🔴
Structural	External Contract ↔ Internal Schema	🔴
Structural	Mixed Type Contract	🟡
Structural	Framework Reimplementation	🟡
Structural	Mutable-Constant	🟡
Structural	Interface Constructor Lock-in	🔴
Readability	Naming Quality	🟡
Readability	Cognitive Complexity	🟡
Readability	Function Length	🟡
Readability	Comment Quality	💡
Readability	Opaque External Identifiers	🟡
Readability	Magic Values	🟡
Test	Test Case Misclassification	🟡
Test	Fragile Test	🔴
Test	Optional-Parameter Laziness	🔴
Community	Duplicated Code	🔴
Community	Dead Code	🟡
Community	Mutable Global State	🔴
Community	Leaky Abstraction	🟡
Community	Message Chains	🟡
Community	Speculative Generality	🟡
Community	Temporary Field	🟡
Community	Data Class / Anemic Model	🟡
Community	Poltergeist	💡
Community	Cyclomatic Complexity	🟡
Community	Parallel Inheritance	🟡
Community	Inconsistent Error Handling	🔴
Community	Lazy Class	💡
Community	Alternative Class Interfaces	🟡
Community	Incomplete Library Class	🟡