Refactoring code smells

Code Smells

A code smell is a surface indication that usually corresponds to a deeper problem in the system. Smells don't always indicate a bug, but they suggest that the design could be improved.

When to Use This Skill

Triggers:

• You need to identify what's wrong with a piece of code
• Code review reveals structural concerns but you can't name the problem
• You want a systematic checklist of what to look for in a codebase
• You're planning a refactoring session and need to prioritize

Don't Triggers:

• You already know the smell and need the specific refactoring technique (use the appropriate sub-skill instead)
• You're writing new code from scratch
• The issue is a bug, not a design problem

The Code Smells Table

1. Duplicated Code

Signal: The same code structure appears in more than one place.

# BEFORE — duplicated logic
def calculate_us_tax(income):
    if income < 10000:
        return income * 0.1
    elif income < 50000:
        return income * 0.2
    else:
        return income * 0.3

def calculate_uk_tax(income):
    if income < 10000:
        return income * 0.1
    elif income < 50000:
        return income * 0.2
    else:
        return income * 0.3

Refactoring: Extract Method, Extract Superclass, Pull Up Method, Form Template Method.

---

2. Long Method

Signal: A method has many lines, or you need comments to explain sections within it.

# BEFORE — long method doing too much
def process_order(order):
    # validate
    if not order.items:
        raise ValueError("Empty order")
    if not order.customer:
        raise ValueError("No customer")
    # calculate total
    total = 0
    for item in order.items:
        total += item.price * item.quantity
        if item.quantity > 10:
            total *= 0.95
    # apply discount
    if order.customer.is_vip:
        total *= 0.9
    # charge
    payment_gateway.charge(order.customer, total)
    # notify
    email_service.send(order.customer.email, f"Order charged: {total}")

Refactoring: Extract Method, Replace Temp with Query, Introduce Parameter Object, Preserve Whole Object.

---

3. Large Class

Signal: A class has too many fields or methods. It tries to do too many things.

# BEFORE — large class with too many responsibilities
class Employee:
    def __init__(self, name, address, phone, salary, tax_id):
        self.name = name
        self.address = address
        self.phone = phone
        self.salary = salary
        self.tax_id = tax_id
        self.projects = []

    def calculate_pay(self): ...
    def calculate_tax(self): ...
    def calculate_bonus(self): ...
    def generate_report(self): ...
    def save_to_database(self): ...
    def send_email(self, message): ...
    def add_project(self, project): ...
    def remove_project(self, project): ...

Refactoring: Extract Class, Extract Subclass, Extract Interface.

---

4. Long Parameter List

Signal: A function takes many parameters.

# BEFORE — too many parameters
def create_user(name, email, phone, address, city, state, zip_code, country, role, department):
    ...

Refactoring: Replace Parameter with Method Call, Preserve Whole Object, Introduce Parameter Object.

# AFTER — parameter object
@dataclass
class UserParams:
    name: str
    email: str
    phone: str
    address: str
    city: str
    state: str
    zip_code: str
    country: str
    role: str
    department: str

def create_user(params: UserParams):
    ...

---

5. Divergent Change

Signal: One class is commonly changed for different reasons.

# BEFORE — class changes for both formatting AND data access reasons
class Report:
    def __init__(self, data):
        self.data = data

    def fetch_data(self, query): ...       # changes when data source changes
    def format_html(self): ...             # changes when output format changes
    def format_csv(self): ...              # changes when output format changes
    def save_to_file(self, path): ...      # changes when storage changes

Refactoring: Extract Class.

# AFTER — separated responsibilities
class ReportData:
    def fetch(self, query): ...

class ReportFormatter:
    def format_html(self, data): ...
    def format_csv(self, data): ...

class ReportStorage:
    def save_to_file(self, data, path): ...

---

6. Shotgun Surgery

Signal: Every time you make a change, you have to modify many classes.

# BEFORE — adding a new tax type requires changes in multiple files
# order.py
def calculate_total(self):
    return self.subtotal + self.subtotal * TaxRates.get_rate("standard")

# invoice.py
def apply_tax(self):
    self.tax = self.amount * TaxRates.get_rate("standard")

# receipt.py
def calculate_tax(self):
    return self.total * TaxRates.get_rate("standard")

Refactoring: Move Method, Move Field, Inline Class.

# AFTER — tax logic in one place
class TaxCalculator:
    def calculate(self, amount, tax_type="standard"):
        rate = self._get_rate(tax_type)
        return amount * rate

---

7. Feature Envy

Signal: A method uses more data from another class than its own.

# BEFORE — method is more interested in Order than its own class
class Customer:
    def get_shipping_cost(self, order):
        base_cost = order.total * 0.1
        if order.is_international:
            base_cost += 50
        if order.weight > 10:
            base_cost += 20
        return base_cost

Refactoring: Move Method.

# AFTER — behavior moved to the class whose data it uses
class Order:
    def get_shipping_cost(self):
        base_cost = self.total * 0.1
        if self.is_international:
            base_cost += 50
        if self.weight > 10:
            base_cost += 20
        return base_cost

---

8. Data Clumps

Signal: The same group of data items (fields, parameters) appears together repeatedly.

# BEFORE — x, y, z always appear together
class Renderer:
    def draw_point(self, x, y, z): ...
    def calculate_distance(self, x1, y1, z1, x2, y2, z2): ...
    def translate(self, x, y, z, dx, dy, dz): ...

Refactoring: Extract Class, Introduce Parameter Object, Preserve Whole Object.

# AFTER — data clump becomes a first-class concept
@dataclass
class Point3D:
    x: float
    y: float
    z: float

class Renderer:
    def draw_point(self, point: Point3D): ...
    def calculate_distance(self, p1: Point3D, p2: Point3D): ...
    def translate(self, point: Point3D, delta: Point3D): ...

---

9. Primitive Obsession

Signal: Using primitive types (strings, ints) to represent domain concepts.

# BEFORE — primitives represent domain concepts
class Order:
    def __init__(self):
        self.status = "pending"  # string used as enum
        self.currency = "USD"    # string used as value object
        self.discount = 0.1      # float used as percentage

Refactoring: Replace Data Value with Object, Replace Type Code with Subclasses, Replace Type Code with State/Strategy.

# AFTER — domain concepts have their own types
from enum import Enum

class OrderStatus(Enum):
    PENDING = "pending"
    CONFIRMED = "confirmed"
    SHIPPED = "shipped"

@dataclass(frozen=True)
class Currency:
    code: str

@dataclass(frozen=True)
class Percentage:
    value: float  # 0.0 to 1.0

---

10. Switch Statements

Signal: A switch/match statement or chain of if-elif that grows every time a new type is added.

# BEFORE — switch on type
class Employee:
    def __init__(self, emp_type):
        self.emp_type = emp_type

def calculate_bonus(employee):
    if employee.emp_type == "engineer":
        return employee.salary * 0.1
    elif employee.emp_type == "manager":
        return employee.salary * 0.2
    elif employee.emp_type == "sales":
        return employee.salary * 0.15
    else:
        return 0

Refactoring: Replace Conditional with Polymorphism, Replace Type Code with Subclasses.

# AFTER — polymorphism eliminates the switch
class Employee(ABC):
    @abstractmethod
    def calculate_bonus(self) -> float: ...

class Engineer(Employee):
    def calculate_bonus(self) -> float:
        return self.salary * 0.1

class Manager(Employee):
    def calculate_bonus(self) -> float:
        return self.salary * 0.2

class Sales(Employee):
    def calculate_bonus(self) -> float:
        return self.salary * 0.15

---

11. Parallel Inheritance Hierarchies

Signal: Every time you make a subclass of one class, you must also make a subclass of another.

# BEFORE — adding a new vehicle type requires a new controller type
class Vehicle: ...
class Car(Vehicle): ...
class Truck(Vehicle): ...

class VehicleController: ...
class CarController(VehicleController): ...
class TruckController(VehicleController): ...

Refactoring: Move Method, Move Field — collapse one hierarchy by having the other reference it.

---

12. Lazy Class

Signal: A class that doesn't do enough to justify its existence.

# BEFORE — class just wraps a string
class CustomerName:
    def __init__(self, name: str):
        self.name = name

    def get_name(self):
        return self.name

Refactoring: Inline Class or Collapse Hierarchy. If a subclass is the lazy class, use Collapse Hierarchy.

# AFTER — inline the class, use the string directly
class Customer:
    def __init__(self, name: str):
        self.name = name

---

13. Speculative Generality

Signal: Code exists "just in case" — abstract classes nobody extends, methods nobody calls, parameters nobody uses.

# BEFORE — over-engineered for future needs
class DataProcessor(ABC):
    @abstractmethod
    def process(self, data): ...

    @abstractmethod
    def validate(self, data): ...

    @abstractmethod
    def transform(self, data): ...  # nobody uses this yet

    def supports_batch(self) -> bool:  # always returns False
        return False

Refactoring: Collapse Hierarchy, Inline Class, Remove Parameter, Rename Method.

---

14. Temporary Field

Signal: A field is only used in certain circumstances or certain methods.

# BEFORE — target_customer only used during a specific calculation
class OrderProcessor:
    def __init__(self):
        self.target_customer = None  # only set during discount calc

    def calculate_discount(self, customer):
        self.target_customer = customer
        discount = self._compute_discount()
        self.target_customer = None
        return discount

    def _compute_discount(self):
        if self.target_customer and self.target_customer.is_vip:
            return 0.2
        return 0.0

Refactoring: Extract Class, Introduce Null Object.

# AFTER — extracted to its own context
class DiscountCalculator:
    def calculate(self, customer):
        if customer.is_vip:
            return 0.2
        return 0.0

---

15. Message Chains

Signal: A series of calls like a.get_b().get_c().get_d().

# BEFORE — reaching through multiple objects
class Order:
    def get_customer(self): ...

# Client code:
order.get_customer().get_address().get_city()

Refactoring: Hide Delegate, Extract Method.

# AFTER — provide a shortcut
class Order:
    def get_customer(self): ...
    def get_customer_city(self):
        return self.get_customer().get_address().get_city()

---

16. Middle Man

Signal: A class exists mostly to delegate to another class.

# BEFORE — Person just delegates everything to EmployeeRecord
class Person:
    def __init__(self, employee_record):
        self._employee_record = employee_record

    def get_name(self):
        return self._employee_record.get_name()

    def get_phone(self):
        return self._employee_record.get_phone()

    def get_department(self):
        return self._employee_record.get_department()

Refactoring: Remove Middle Man, Inline Class.

# AFTER — clients talk to EmployeeRecord directly
class Person:
    def __init__(self, employee_record):
        self.employee_record = employee_record

---

17. Inappropriate Intimacy

Signal: One class reaches into another class's internals (accessing private fields, knowing too much about implementation).

# BEFORE — Order directly manipulates Customer internals
class Order:
    def apply_discount(self):
        if self.customer._loyalty_points > 1000:
            self.total *= 0.9
            self.customer._loyalty_points -= 500

Refactoring: Move Method, Move Field, Change Bidirectional Association to Unidirectional, Extract Class, Hide Delegate.

# AFTER — Customer manages its own loyalty
class Customer:
    def apply_loyalty_discount(self, total):
        if self._loyalty_points > 1000:
            self._loyalty_points -= 500
            return total * 0.9
        return total

class Order:
    def apply_discount(self):
        self.total = self.customer.apply_loyalty_discount(self.total)

---

18. Alternative Classes with Different Interfaces

Signal: Two classes do the same thing but have different method names.

# BEFORE — same concept, different interfaces
class CustomerValidator:
    def validate(self, customer): ...

class ClientChecker:
    def check(self, client): ...  # does the same thing

Refactoring: Rename Method, Move Method, Extract Superclass.

# AFTER — unified interface
class Validator(ABC):
    @abstractmethod
    def validate(self, entity): ...

class CustomerValidator(Validator):
    def validate(self, customer): ...

class ClientValidator(Validator):
    def validate(self, client): ...

---

19. Incomplete Library Class

Signal: A library class is almost what you need but lacks a method you need.

# BEFORE — need to extend a library class with a missing method
# The library's StringUtils has left_trim but no left_trim_and_pad

Refactoring: Introduce Foreign Method, Introduce Local Extension.

# AFTER — local extension
class ExtendedStringUtils(StringUtils):
    @staticmethod
    def left_trim_and_pad(s: str, width: int) -> str:
        return s.lstrip().ljust(width)

---

20. Data Class

Signal: A class has fields and getters/setters but no real behavior.

# BEFORE — data class with no behavior
class Point:
    def __init__(self, x, y):
        self._x = x
        self._y = y

    def get_x(self): return self._x
    def get_y(self): return self._y
    def set_x(self, x): self._x = x
    def set_y(self, y): self._y = y

Refactoring: Move Method, Encapsulate Field, Remove Setting Method, Extract Method.

# AFTER — behavior lives with the data
class Point:
    def __init__(self, x, y):
        self.x = x
        self.y = y

    def distance_to(self, other: "Point") -> float:
        return ((self.x - other.x) ** 2 + (self.y - other.y) ** 2) ** 0.5

    def translate(self, dx, dy):
        return Point(self.x + dx, self.y + dy)

---

21. Refused Bequest

Signal: A subclass doesn't use or overrides most of what it inherits from its parent.

# BEFORE — Stack shouldn't inherit from List
class EnhancedList(list):
    def push(self, item):
        self.append(item)

    def pop_item(self):
        return self.pop()

Refactoring: Replace Inheritance with Delegation.

# AFTER — delegation instead of inheritance
class Stack:
    def __init__(self):
        self._items = []

    def push(self, item):
        self._items.append(item)

    def pop(self):
        return self._items.pop()

    def is_empty(self):
        return len(self._items) == 0

---

22. Comments

Signal: A comment exists to explain confusing code. The comment is a deodor — it masks the smell.

# BEFORE — comment explains bad code
def calculate(order):
    # Apply 15% discount if customer has been with us for more than 2 years
    # and has placed more than 10 orders
    if order.customer.tenure_days > 730 and order.total_orders > 10:
        return order.total * 0.85
    return order.total

Refactoring: Extract Method, Introduce Explaining Variable, Rename Method.

# AFTER — the code explains itself
def calculate(order):
    if self._qualifies_for_loyalty_discount(order.customer):
        return order.total * (1 - LOYALTY_DISCOUNT_RATE)
    return order.total

def _qualifies_for_loyalty_discount(self, customer):
    return customer.tenure_years >= 2 and customer.total_orders > 10

---

Quick Reference — Smell → Refactoring Map

Smell	Primary Refactorings
Duplicated Code	Extract Method, Extract Superclass, Pull Up Method
Long Method	Extract Method, Replace Temp with Query
Large Class	Extract Class, Extract Subclass, Extract Interface
Long Parameter List	Introduce Parameter Object, Preserve Whole Object
Divergent Change	Extract Class
Shotgun Surgery	Move Method, Move Field, Inline Class
Feature Envy	Move Method
Data Clumps	Extract Class, Introduce Parameter Object
Primitive Obsession	Replace Data Value with Object
Switch Statements	Replace Conditional with Polymorphism
Parallel Inheritance Hierarchies	Move Method, Move Field
Lazy Class	Inline Class, Collapse Hierarchy
Speculative Generality	Collapse Hierarchy, Inline Class, Remove Parameter
Temporary Field	Extract Class, Introduce Null Object
Message Chains	Hide Delegate, Extract Method
Middle Man	Remove Middle Man, Inline Class
Inappropriate Intimacy	Move Method, Extract Class, Hide Delegate
Alternative Classes	Rename Method, Extract Superclass
Incomplete Library Class	Introduce Foreign Method, Introduce Local Extension
Data Class	Move Method, Encapsulate Field
Refused Bequest	Replace Inheritance with Delegation
Comments	Extract Method, Rename Method, Introduce Explaining Variable

---

Source: Martin Fowler, "Refactoring: Improving the Design of Existing Code" (2nd Edition)