code-smell-catalog
verifiedf2e7a39b-5f2a-44ae-9169-48dc7533c16b
Use when identifying code smells and their fixes — smell→refactor mapping (Long Function→Extract, Primitive Obsession→Value Object, Duplication→Parameterize), with detection signals and worked examples.
Metadata
Skill file
# Code Smell Catalog
**Use when** reviewing or cleaning code and you need to name the smell and know the mechanical fix. Each smell has a detection signal, a refactoring, and a worked before/after.
## The Smell → Refactor Table
| Smell | Detection Signal | Refactoring |
|-------|------------------|-------------|
| **Long Function** | >20-30 lines, multiple responsibilities | Extract Function |
| **Long Parameter List** | >4-5 params | Introduce Parameter Object |
| **Primitive Obsession** | Strings/ints used for concepts (currency, status) | Introduce Value Object / Enum |
| **Duplicated Code** | Same block in 2+ places | Extract + Parameterize |
| **Feature Envy** | Method uses another class's data more than its own | Move Method |
| **Shotgun Surgery** | One change touches many classes | Move Method/Field |
| **Divergent Change** | One class changes for many reasons | Extract Class |
| **Large Class** | >300 lines or >10 methods | Extract Class |
| **Switch Statements** | `if/else` or `switch` on a type code | Replace with Polymorphism |
| **Comments** | Comments explain WHAT not WHY | Rename/Extract (let code explain) |
## Worked Before/After: Top-5 Smells
### 1. Long Function → Extract Function
```python
# SMELL: 40 lines, does three things
def process_order(order):
total = 0
for item in order.items:
total += item.price * item.quantity
total *= 1.08
if total > 10000:
raise ValueError("too large")
if not order.customer_id:
raise ValueError("no customer")
db.orders.insert(order_id=order.id, total=total)
return total
# FIX: Extract functions, one responsibility each
def process_order(order):
total = calculate_total(order)
validate(order, total)
db.orders.insert(order_id=order.id, total=total)
return total
def calculate_total(order):
return sum(i.price * i.quantity for i in order.items) * 1.08
def validate(order, total):
if total > 10000: raise ValueError("too large")
if not order.customer_id: raise ValueError("no customer")
```
### 2. Primitive Obsession → Value Object
```python
# SMELL: strings/ints for domain concepts
def charge(amount, currency, country):
if currency == "USD" and country == "US": ...
if currency == "EUR" and country in ("DE", "FR", "IT"): ...
# FIX: Introduce a Currency enum / Money value object
from enum import Enum
class Currency(Enum):
USD = "USD"
EUR = "EUR"
def charge(amount, currency: Currency, country):
if currency is Currency.USD and country == "US": ...
```
### 3. Duplication → Extract + Parameterize
```python
# SMELL: same logic, different constants
def is_us_citizen(user): return user.country == "US" and user.age >= 18
def is_eu_citizen(user): return user.country == "EU" and user.age >= 18
# FIX: Parameterize the difference
def is_citizen(user, region): return user.country == region and user.age >= 18
# Or better: encode the invariant once
def is_adult(user): return user.age >= 18
def is_citizen(user, region): return user.country == region and is_adult(user)
```
### 4. Feature Envy → Move Method
```python
# SMELL: process() uses user's data more than its own
class Report:
def process(self, user):
return f"{user.first_name} {user.last_name} from {user.city}"
# FIX: Move the behavior to User
class User:
def full_name(self): return f"{self.first_name} {self.last_name}"
def display(self): return f"{self.full_name()} from {self.city}"
class Report:
def process(self, user):
return user.display()
```
### 5. Switch Statement → Polymorphism
```python
# SMELL: if/elif on a type code
def get_area(shape):
if shape.type == "circle": return 3.14 * shape.r ** 2
elif shape.type == "square": return shape.side ** 2
elif shape.type == "rect": return shape.w * shape.h
# FIX: Polymorphism
class Circle:
def area(self): return 3.14 * self.r ** 2
class Square:
def area(self): return self.side ** 2
class Rect:
def area(self): return self.w * self.h
def get_area(shape): return shape.area()
```
## Detection Signals (quantitative)
```bash
# Find long functions (Python, heuristic):
# grep for functions spanning many lines
awk '/def /{start=NR} /^$/{if(NR-start>25) print start" to "NR}' src/*.py
# Find duplicated blocks (simplified):
# Use a tool or manual review of repeated patterns
grep -rn "total \* 1.08" src/ # if 3+ hits -> duplication
# Fan-out (how many things a class depends on):
# PyCharm: Analyze > Inspect Code > Dependency Analysis
```
| Signal | Threshold | Likely Smell |
|--------|-----------|--------------|
| Function length | >25 lines | Long Function |
| Parameter count | >4 | Long Parameter List |
| Class size | >300 lines / >10 methods | Large Class |
| `if/elif` chain length | >3 branches on same var | Switch Statement |
| Method accessing another class | >3 fields of other class | Feature Envy |
## Guardrails
- **Not every smell needs fixing.** A 30-line function that reads top-to-bottom may be fine. Smells are signals, not bugs.
- **Don't fix smells without test coverage.** Add characterization tests first (see working-with-legacy-code).
- **One smell at a time.** Fix Long Function, verify, then move to the next.
- **Smell-driven churn without coverage is risk, not improvement.** If you can't prove behavior is preserved, you're not refactoring — you're gambling.
- Don't apply the catalog mechanically. Judgment: is this smell actually hurting maintainability at THIS scale?
## Pitfalls
| Pitfall | Fix |
|---------|-----|
| Fixing smells that are fine at this scale | Weigh: is it hurting readability? If not, leave it |
| Refactoring without tests | Characterization tests first (working-with-legacy-code) |
| Over-extracting (100 tiny functions) | Balance: extract when it names a real concept |
| Polishing code no one reads | Prioritize smells in hot paths / frequently-changed code |
| Applying all refactorings at once | One smell → one refactoring → run tests → commit |
## Verify / Checklist
- [ ] Each smell identified with a detection signal (not vibes)
- [ ] Smell mapped to the correct refactoring from the table
- [ ] Tests green before and after each refactoring
- [ ] One refactoring per commit
- [ ] Judgment applied (not every smell fixed)
- [ ] No behavior change (tests unchanged except new characterization tests)Attached files
No attached files.