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.