code-smells

name	code-smells
description	Detect code smells based on refactoring.guru catalog - use when analyzing code for anti-patterns and refactoring opportunities

Code Smells Detection

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

This skill is a **reference catalog** for code smell detection. It is used by: - `pr-review` Agent 3 (Code Smells Agent) during PR review - `planning-refactor` when identifying improvement opportunities - Standalone code quality analysis

When detecting smells, report findings in structured format with:

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

When to Use

Context	Trigger
PR Review	Invoked by pr-review Code Smells Agent
Refactoring Planning	Before planning-refactor to identify targets
Code Quality Audit	User asks to analyze code quality or find anti-patterns
Technical Debt Assessment	Identifying areas needing improvement

Severity Levels

Severity	Score	Impact	Examples
Critical	100	Causes bugs, crashes, security issues	Feature Envy causing null dereference
Must Fix	80	Significantly harms maintainability	God Class, Shotgun Surgery
Should Fix	50	Reduces code quality	Long Method, Duplicate Code
Nitpick	20	Minor improvement opportunity	Lazy Class, Comments

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:

Technique	When to Use
Extract Method	Split into smaller methods with descriptive names
Replace Temp with Query	Local variables interfering with extraction
Introduce Parameter Object	Many parameters passed between methods
Decompose Conditional	Complex if/else chains
Replace Method with Method Object	When extraction is too complex

---

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:

Technique	When to Use
Extract Class	Separate distinct behavioral components
Extract Subclass	Behavior used only in some cases
Extract Interface	Define contract for operations
Duplicate Observed Data	GUI classes with domain logic

---

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:

Technique	When to Use
Replace Data Value with Object	Group primitives into dedicated classes
Introduce Parameter Object	Primitives in method parameters
Preserve Whole Object	Passing extracted values instead of object
Replace Type Code with Class/Subclasses/State	Constants simulating types
Replace Array with Object	Using arrays with string keys

---

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:

Technique	When to Use
Replace Parameter with Method Call	Parameter is result of another call
Preserve Whole Object	Passing multiple fields from same object
Introduce Parameter Object	Group related parameters

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:

Technique	When to Use
Extract Class	For class fields
Introduce Parameter Object	For method parameters
Preserve Whole Object	Pass object instead of extracting values

---

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:

Technique	When to Use
Replace Conditional with Polymorphism	When switch determines behavior
Replace Type Code with Subclasses	Type codes with distinct behavior
Replace Type Code with State/Strategy	Behavior varies at runtime
Replace Parameter with Explicit Methods	Simple value-based switches
Introduce Null Object	Null checks in conditionals

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:

Technique	When to Use
Extract Class	Move field and related code to separate class
Replace Method with Method Object	Algorithm needs many temp fields
Introduce Null Object	Replace null checks with null object pattern

---

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:

Technique	When to Use
Replace Inheritance with Delegation	Wrong hierarchy, use composition
Extract Superclass	Create proper abstraction for shared behavior

---

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:

Technique	When to Use
Rename Method	Align method names to common interface
Move Method	Consolidate implementations
Extract Superclass	Share common behavior
Delete redundant class	After consolidation

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:

Technique	When to Use
Extract Class	Separate each concern into its own class
Extract Superclass/Subclass	When classes share some behavior

---

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:

Technique	When to Use
Move Method/Field	Consolidate scattered responsibility
Inline Class	After consolidation, remove empty shells

---

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:

Technique	When to Use
Move Method/Field	Make one hierarchy reference the other
Eliminate redundant hierarchy	After consolidation

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:

Technique	When to Use
Extract Variable	Complex expression needs explanation
Extract Method	Use comment text as method name
Rename Method	Method name should make comment unnecessary
Introduce Assertion	Document required state

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:

Technique	When to Use
Extract Method	Within single class
Pull Up Method	In sibling subclasses
Form Template Method	Similar but not identical algorithms
Extract Superclass	Across unrelated classes
Extract Class	When hierarchy inappropriate
Consolidate Conditional Expression	Duplicate conditions

---

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:

Technique	When to Use
Inline Class	Merge into another class
Collapse Hierarchy	Merge minimal subclass with parent

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:

Technique	When to Use
Encapsulate Field	Make fields private
Encapsulate Collection	For collections
Move Method	Bring behavior from clients into class
Remove Setting Method	After class gains behavior
Hide Method	For overly permissive accessors

---

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:

Technique	When to Use
Delete	Remove unused code
Remove Parameter	Unused parameters
Inline Class/Collapse Hierarchy	Unused classes

---

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:

Technique	When to Use
Collapse Hierarchy	Unnecessary abstract classes
Inline Class	Unnecessary delegation
Inline Method	Unused methods
Remove Parameter	Unused parameters

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:

Technique	When to Use
Move Method	Move entire method to target class
Extract Method	Move only envious part

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:

Technique	When to Use
Move Method/Field	Put code where data lives
Extract Class	Separate shared parts
Hide Delegate	Reduce direct knowledge
Change Bidirectional to Unidirectional	Simplify association
Replace Delegation with Inheritance	When intimacy is with subclass

---

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:

Technique	When to Use
Hide Delegate	Shorten chain at each step
Extract Method + Move Method	Get what you need from chain start

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:

Technique	When to Use
Remove Middle Man	Call delegate directly

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:

Technique	When to Use
Introduce Foreign Method	Add few methods
Introduce Local Extension	Add substantial functionality

---

Quick Reference: Detection Thresholds

Smell	Threshold	Severity
Long Method	>50 lines	Should Fix (50)
Long Method	>100 lines	Must Fix (80)
Large Class	>500 lines	Should Fix (50)
Large Class	>1000 lines	Must Fix (80)
Long Parameter List	>4 params	Should Fix (50)
Long Parameter List	>6 params	Must Fix (80)
Deep Nesting	>3 levels	Should Fix (50)
Deep Nesting	>5 levels	Must Fix (80)
Magic Numbers	Any unexplained literal	Should Fix (50)
Duplicate Code	>10 lines identical	Should Fix (50)
Feature Envy	>3 external getters	Should Fix (50)
Message Chain	>3 calls	Should Fix (50)
Shotgun Surgery	>5 classes touched	Must Fix (80)
God Class	Multiple responsibilities	Must Fix (80)

Integration with Other Skills

Scenario	Next Skill
Found smells during PR review	Continue pr-review with findings
Found smells, need refactoring	Invoke planning-refactor
Need to understand code first	Invoke exploration-explore-code
Ready to apply specific technique	Invoke refactoring-techniques for step-by-step guidance
Ready to fix Long Method	Use Extract Method from refactoring-techniques
Ready to fix Feature Envy	Use Move Method from refactoring-techniques

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