Code Smells

Code Smells Detection

Comprehensive catalog of code smells for detecting anti-patterns and identifying refactoring opportunities. Based on the Refactoring.Guru Code Smells Catalog.

When detecting smells, report findings in structured format with:

• File:Line location
• Smell name and category
• Severity score
• Suggested refactoring

When to Use

Severity Levels

Output Format

When reporting code smells, use this structured format:

| File:Line | Smell | Category | Severity | Suggested Refactoring |
|-----------|-------|----------|----------|----------------------|
| handler.cpp:120-195 | Long Method (75 lines) | Bloater | Should Fix (50) | Extract Method: split into validateRequest(), processData(), buildResponse() |
| config.cpp:45 | Magic Number | Bloater | Should Fix (50) | Replace Magic Number: `const int MAX_RETRIES = 42;` |

Category 1: Bloaters

Code that has grown to excessive proportions, making it hard to work with.

Long Method

Threshold: >10 lines warrants questions, >50 lines is definite smell

Signs:

• Method contains too many lines of code
• You need to scroll to see the whole method
• Method does multiple things

Why it's bad:

• Hard to understand and maintain
• Hides duplicate code
• Makes testing difficult
• Long methods accumulate like "Hotel California" - easy to add, hard to remove

Detection criteria:

- Lines > 50: Should Fix (50)
- Lines > 100: Must Fix (80)
- Method does more than one thing (multiple responsibilities)
- Requires extensive comments to explain

Refactoring:

Large Class

Threshold: Too many fields, methods, or lines (>500 lines warrants review)

Signs:

• Class contains excessive fields/methods/lines
• Class has multiple unrelated responsibilities
• You struggle to summarize what the class does in one sentence

Why it's bad:

• Cognitive overload (too many attributes to track)
• Code duplication within the class
• Violates Single Responsibility Principle

Detection criteria:

- Lines > 500: Should Fix (50)
- Lines > 1000: Must Fix (80)
- >10 public methods: Review for SRP violation
- >15 fields: Likely doing too much
- Multiple unrelated method groups

Refactoring:

Primitive Obsession

Signs:

• Using primitives instead of small objects (money, phone numbers, ranges)
• Constants encoding information (USER_ADMIN_ROLE = 1)
• String constants as field names in arrays
• Using int for IDs instead of typed wrapper

Why it's bad:

• No type safety (passing wrong int to function)
• Related operations scattered across codebase
• No place to add validation or behavior
• Duplicate code for handling the primitive

Detection criteria:

- Multiple functions operating on same primitive group: Should Fix (50)
- Constants simulating types: Should Fix (50)
- No domain objects for business concepts (money, date ranges, coordinates): Should Fix (50)

Refactoring:

Long Parameter List

Threshold: >3-4 parameters

Signs:

• Method has more than 3-4 parameters
• Parameters are passed through multiple method calls
• Hard to remember parameter order
• Boolean parameters for mode switching

Why it's bad:

• Difficult to understand method signature
• Easy to pass wrong arguments
• Often indicates the method is doing too much
• Creates tight coupling

Detection criteria:

- 4-5 parameters: Should Fix (50)
- 6+ parameters: Must Fix (80)
- Boolean "flag" parameters: Should Fix (50)
- Parameters from same object: Should Fix (50)

Refactoring:

When to ignore: Creating dependencies between classes may be worse than long parameter list.

Data Clumps

Signs:

• Same group of variables appears in multiple places
• Parameters that always travel together
• Removing one variable makes others meaningless

Why it's bad:

• Code duplication
• No single place for operations on the data
• Easy to pass incomplete groups

Detection criteria:

- Same 3+ parameters in multiple method signatures: Should Fix (50)
- Same fields grouped in multiple classes: Should Fix (50)
- Database connection parameters passed separately: Should Fix (50)

Refactoring:

Category 2: Object-Orientation Abusers

Incomplete or incorrect application of object-oriented programming principles.

Switch Statements

Signs:

• Complex switch operator or if-else chain based on type
• Same switch logic scattered across multiple methods
• Adding a new case requires changes in many places

Why it's bad:

• Violates Open/Closed Principle
• Duplicate switch logic across codebase
• Easy to forget updating all switches
• Rare use of switch is hallmark of good OOP

Detection criteria:

- Switch on type/enum with behavior: Should Fix (50)
- Same switch in multiple places: Must Fix (80)
- Switch with >5 cases with different behavior: Should Fix (50)

Refactoring:

When to ignore: Factory patterns legitimately use switch to create objects.

Temporary Field

Signs:

• Fields only used in certain circumstances
• Fields remain empty/null most of the time
• Field existence confuses readers

Why it's bad:

• Developers expect fields to always be meaningful
• Requires null checks scattered through code
• Hides actual dependencies

Detection criteria:

- Field used in <25% of methods: Should Fix (50)
- Field only set in one method and used in another: Should Fix (50)
- Fields for algorithm-specific data: Should Fix (50)

Refactoring:

Refused Bequest

Signs:

• Subclass uses only some parent methods/properties
• Inherited methods throw exceptions or do nothing
• Subclass doesn't follow Liskov Substitution Principle

Why it's bad:

• Confusing hierarchy (Dog inherits from Chair?)
• Breaking substitutability
• Indicates wrong abstraction

Detection criteria:

- Override methods with empty body or exception: Should Fix (50)
- Subclass ignores >50% of parent interface: Should Fix (50)
- Inheritance for code reuse, not "is-a": Should Fix (50)

Refactoring:

Alternative Classes with Different Interfaces

Signs:

• Two classes do the same thing with different method names
• Parallel implementations that could be unified
• Duplicate functionality discovered during review

Why it's bad:

• Code duplication across classes
• Confusion about which class to use
• Maintenance burden

Detection criteria:

- Classes with same purpose, different interface: Should Fix (50)
- Duplicate logic in differently-named methods: Should Fix (50)

Refactoring:

When to ignore: Classes in different libraries with independent versioning.

Category 3: Change Preventers

Issues that require changes in multiple places when a single change is needed.

Divergent Change

Signs:

• Single class needs changes for multiple unrelated reasons
• Adding new feature requires changing many methods in one class
• Class has multiple "areas" of responsibility

Why it's bad:

• Single class handles too many concerns
• Any change risks breaking unrelated functionality
• Violates Single Responsibility Principle

Detection criteria:

- Class changes for >2 unrelated reasons: Should Fix (50)
- Class has distinct "sections" of functionality: Should Fix (50)
- Methods naturally group into separate concerns: Should Fix (50)

Refactoring:

Shotgun Surgery

Signs:

• Single change requires edits to many different classes
• Adding a feature means touching 5+ files
• Opposite of Divergent Change

Why it's bad:

• Easy to miss a required change
• High chance of introducing bugs
• Expensive maintenance

Detection criteria:

- Feature addition touches 5+ classes: Must Fix (80)
- Single responsibility scattered across classes: Must Fix (80)
- "Overzealous" previous separation causing this: Must Fix (80)

Refactoring:

Parallel Inheritance Hierarchies

Signs:

• Creating subclass in one hierarchy requires subclass in another
• Class prefixes match across hierarchies
• Mirrored class structures

Why it's bad:

• Duplicate maintenance
• Growing complexity as hierarchies expand
• Easy to forget creating matching class

Detection criteria:

- Two hierarchies with matching structures: Should Fix (50)
- Adding to one requires adding to another: Should Fix (50)
- Prefixes like "XHandler/XFactory": Should Fix (50)

Refactoring:

When to ignore: If deduplication makes code uglier, keep the parallel structure.

Category 4: Dispensables

Pointless and unneeded elements whose absence would make code cleaner.

Comments

Signs:

• Method filled with explanatory comments
• Comments describe WHAT instead of WHY
• Comments are outdated or incorrect
• Comments compensate for unclear code

Why it's bad:

• "The best comment is a good name for a method or class"
• Comments can become outdated and misleading
• Indicates code needs refactoring, not explaining

Detection criteria:

- Comment explains what code does (not why): Nitpick (20)
- Commented-out code: Should Fix (50)
- Comment contradicts code: Must Fix (80)
- TODOs without ticket references: Nitpick (20)

Refactoring:

When acceptable: Comments explaining WHY (design decisions, complex algorithms, non-obvious constraints).

Duplicate Code

Signs:

• Two code fragments look almost identical
• Same logic in different methods/classes
• Copy-paste programming

Why it's bad:

• Changes must be made in multiple places
• Easy to miss updating all copies
• Increases maintenance burden

Detection criteria:

- Identical code blocks: Should Fix (50)
- Similar code blocks (>80% same): Should Fix (50)
- Copy-paste with minor modifications: Should Fix (50)
- Same algorithm implemented differently: Should Fix (50)

Refactoring:

Lazy Class

Signs:

• Class doesn't do enough to justify existence
• Class was reduced by refactoring and is now minimal
• Class created for anticipated features that never came

Why it's bad:

• Every class costs maintenance
• Adds complexity without value
• Cognitive overhead for developers

Detection criteria:

- Class with <3 methods: Nitpick (20)
- Class that just delegates to another: Nitpick (20)
- Subclass with minimal additions: Nitpick (20)

Refactoring:

When to ignore: Class represents future expansion point.

Data Class

Signs:

• Class contains only fields and getters/setters
• No behavior, just data container
• Other classes operate on its data

Why it's bad:

• Violates encapsulation
• Logic operating on data is scattered
• Can't enforce invariants

Detection criteria:

- Class with only fields + accessors: Should Fix (50)
- No methods operating on own data: Should Fix (50)
- Other classes doing Feature Envy on this class: Should Fix (50)

Refactoring:

Dead Code

Signs:

• Variable, parameter, field, method, or class no longer used
• Code after return/throw/break
• Conditional branches that never execute

Why it's bad:

• Clutters codebase
• Confuses developers
• May be accidentally activated

Detection criteria:

- Unused variable: Nitpick (20)
- Unused method/function: Should Fix (50)
- Unreachable code: Should Fix (50)
- Commented-out code: Should Fix (50)
- Unused class: Should Fix (50)

Refactoring:

Speculative Generality

Signs:

• Unused class, method, field, or parameter
• Hooks for future features that never came
• Overly abstract code for simple requirements

Why it's bad:

• "Just in case" code adds complexity
• YAGNI - You Aren't Gonna Need It
• Makes code harder to understand

Detection criteria:

- Unused abstraction: Nitpick (20)
- Parameter only used in tests: Nitpick (20)
- Abstract class with single implementation: Nitpick (20)
- Methods that only delegate: Nitpick (20)

Refactoring:

When to ignore: Framework code where users may need the hooks.

Category 5: Couplers

Issues involving excessive coupling between classes or excessive delegation.

Feature Envy

Signs:

• Method accesses data of another object more than its own
• Method is more interested in another class
• Heavy use of another object's getters

Why it's bad:

• Logic is in wrong place
• Violates encapsulation
• Makes refactoring harder

Detection criteria:

- Method uses >3 getters from another object: Should Fix (50)
- Method primarily operates on another class's data: Should Fix (50)
- After data move, operations didn't follow: Should Fix (50)

Refactoring:

When to ignore: Strategy/Visitor patterns intentionally separate behavior from data.

Inappropriate Intimacy

Signs:

• Class uses internal fields/methods of another class
• Two classes spend too much time delving into each other
• Bidirectional dependencies

Why it's bad:

• Tight coupling
• Hard to change one class without affecting the other
• Reduced reusability

Detection criteria:

- Classes accessing each other's private/protected: Must Fix (80)
- Bidirectional association: Should Fix (50)
- Deep knowledge of another class's internals: Should Fix (50)

Refactoring:

Message Chains

Signs:

• Series of calls like a.b().c().d()
• Client depends on navigation structure
• Changes to intermediate classes affect client

Why it's bad:

• Client coupled to entire chain structure
• Any chain change requires client updates
• Violates Law of Demeter

Detection criteria:

- Chain of 3+ method calls: Should Fix (50)
- Chain navigation through multiple objects: Should Fix (50)
- "Train wreck" code: a.getB().getC().getD(): Should Fix (50)

Refactoring:

Caution: Overly aggressive hiding creates Middle Man smell.

Middle Man

Signs:

• Class delegates most work to another class
• Class exists only to forward calls
• No substantial independent behavior

Why it's bad:

• Unnecessary indirection
• Adds complexity without value
• Often results from over-applying Hide Delegate

Detection criteria:

- Class where >50% methods just delegate: Should Fix (50)
- Class with only pass-through methods: Should Fix (50)
- Gradual functionality migration left empty shell: Should Fix (50)

Refactoring:

When to ignore:

• Proxy pattern (intentional delegation)
• Decorator pattern
• Hiding dependencies for testing

Incomplete Library Class

Signs:

• Library doesn't meet your needs
• Can't modify library code
• Working around library limitations

Why it's bad:

• Workarounds scattered across codebase
• Duplicate solutions to same problem
• Fragile code depending on library internals

Detection criteria:

- Multiple workarounds for same library limitation: Should Fix (50)
- Helper functions for library functionality: Nitpick (20)

Refactoring:

Quick Reference: Detection Thresholds

Integration with Other Skills

Cross-Reference: This skill identifies WHAT to fix. The refactoring-techniques skill shows HOW to fix it with 60+ techniques and code examples.

References

• Refactoring.Guru - Code Smells
• Refactoring: Improving the Design of Existing Code by Martin Fowler