akaszubski/refactoring-patterns

Refactoring Patterns

Safe techniques for restructuring code without changing its behavior. Every refactoring follows: test green -> refactor -> test green.

Golden Rule

Never refactor and change behavior in the same commit. Refactoring = same behavior, different structure. Feature work = different behavior. Mixing them makes bugs untraceable.

Pre-Refactoring Checklist

Before any refactoring:

• [ ] Tests exist and pass for the code being refactored
• [ ] You can describe what the code does WITHOUT reading it line by line
• [ ] The refactoring has a clear motivation (not "it could be cleaner")
• [ ] The scope is bounded — you know exactly which files/functions change

Core Refactorings

1. Extract Function

When: A code block does one identifiable thing, or you need to add a comment explaining what a block does.

# BEFORE
def process_order(order):
    # Validate order
    if not order.items:
        raise ValueError("Empty order")
    if order.total < 0:
        raise ValueError("Negative total")
    if not order.customer_id:
        raise ValueError("Missing customer")

    # Apply discounts
    total = order.total
    if order.is_member:
        total *= 0.9
    if len(order.items) > 10:
        total *= 0.95

    return total

# AFTER
def process_order(order):
    validate_order(order)
    return apply_discounts(order)

def validate_order(order):
    if not order.items:
        raise ValueError("Empty order")
    if order.total < 0:
        raise ValueError("Negative total")
    if not order.customer_id:
        raise ValueError("Missing customer")

def apply_discounts(order):
    total = order.total
    if order.is_member:
        total *= 0.9
    if len(order.items) > 10:
        total *= 0.95
    return total

Verification: Run tests. Output must be identical.

2. Inline Function

When: A function's body is as clear as its name, or it's only called once and adds indirection without value.

# BEFORE
def is_valid_age(age):
    return age >= 0

def process(age):
    if is_valid_age(age):
        ...

# AFTER (if is_valid_age is only used once and obvious)
def process(age):
    if age >= 0:
        ...

When NOT to inline: If the function is called from multiple places, or if the name adds clarity that the body doesn't.

3. Rename

When: A name doesn't describe what the thing does, or uses abbreviations/jargon.

# BEFORE
def proc(d, f=True):
    ...
x = get_data()
tmp = transform(x)

# AFTER
def process_invoice(invoice_data, *, validate=True):
    ...
raw_invoices = fetch_invoices()
normalized_invoices = normalize(raw_invoices)

Rules:

• Search for ALL usages before renaming (Grep across entire codebase)
• Update imports, tests, documentation, and config files
• If it's a public API, this is a BREAKING CHANGE — requires version bump

4. Move

When: A function/class is in the wrong module — it's more closely related to another module's concerns.

Process:

1. Grep for all imports of the function/class
2. Move to new location
3. Update all import statements
4. Add re-export from old location if it's a public API (temporary, with deprecation warning)
5. Run tests

5. Simplify Conditionals

When: Nested if/else chains, complex boolean expressions, or repeated condition checks.

# BEFORE: Nested guards
def get_price(product, user):
    if product is not None:
        if product.is_available:
            if user is not None:
                if user.is_member:
                    return product.price * 0.9
                else:
                    return product.price
            else:
                return product.price
        else:
            return None
    else:
        return None

# AFTER: Early returns (guard clauses)
def get_price(product, user):
    if product is None or not product.is_available:
        return None

    if user is not None and user.is_member:
        return product.price * 0.9

    return product.price

6. Decompose Large Functions

When: A function is longer than ~30 lines or has multiple levels of abstraction.

Process:

1. Identify logical sections (often marked by comments or blank lines)
2. Extract each section into a named function
3. The parent function should read like a table of contents
4. Each extracted function should work at one level of abstraction

7. Replace Magic Values

# BEFORE
if response.status_code == 429:
    time.sleep(60)

# AFTER
RATE_LIMIT_STATUS = 429
DEFAULT_RETRY_DELAY_SECONDS = 60

if response.status_code == RATE_LIMIT_STATUS:
    time.sleep(DEFAULT_RETRY_DELAY_SECONDS)

Code Smells That Signal Refactoring

| Smell | Refactoring | |-------|-------------| | Long function (>30 lines) | Extract Function, Decompose | | Deeply nested conditionals | Guard Clauses, Extract Function | | Duplicated code blocks | Extract Function, parameterize | | Feature envy (method uses another class's data more than its own) | Move Method | | Long parameter list (>4 params) | Introduce Parameter Object | | Comments explaining "what" (not "why") | Rename, Extract Function | | Boolean parameters | Split into two functions | | Dead code | Delete it |

Refactoring Safety

Test-First Verification

# 1. Confirm tests pass BEFORE refactoring
python -m pytest tests/ -x --tb=short

# 2. Make the refactoring change

# 3. Confirm tests STILL pass
python -m pytest tests/ -x --tb=short

# 4. Verify no behavioral change
git diff  # Review: structure changes only, no logic changes

When to Stop

• FORBIDDEN: "While I'm here, I'll also..." — scope creep
• FORBIDDEN: Refactoring code you don't have tests for
• FORBIDDEN: Refactoring and adding features in the same commit
• FORBIDDEN: Premature abstraction — three similar blocks is NOT a pattern until you see the fourth

Scale Guidelines

| Scope | Approach | |-------|----------| | Single function | Refactor inline, same commit | | Single file | Refactor in dedicated commit | | Multiple files | Plan first, dedicated branch | | Module/package boundary | ADR required, phased approach |