Refactoring Complex Functions

When a function exceeds McCabe complexity limits (C901), extract helper functions that are pure and return values.

Quick Reference

None

result = transform(data)

mutate(data)

The Anti-Pattern: Void Mutator Functions

NEVER create helper functions that mutate a collection passed as an argument.

# INCORRECT - void mutator anti-pattern
def _add_user_fields(result: dict[str, Any], user: User) -> None:
    """Mutates result dict - THIS IS WRONG."""
    result["name"] = user.name
    result["email"] = user.email
    result["role"] = user.role

def _add_metadata(result: dict[str, Any], metadata: Metadata) -> None:
    """Mutates result dict - THIS IS WRONG."""
    result["created_at"] = metadata.created_at
    result["version"] = metadata.version

def build_response(user: User, metadata: Metadata) -> dict[str, Any]:
    result: dict[str, Any] = {}
    _add_user_fields(result, user)  # Side effect
    _add_metadata(result, metadata)  # Side effect
    return result

Why this is wrong:

• Hidden side effects make code harder to reason about
• Can't compose or test helpers in isolation
• Violates command-query separation
• Makes the data flow invisible

The Correct Pattern: Pure Helpers That Return Values

# CORRECT - pure functions that return values
def _build_user_fields(user: User) -> dict[str, Any]:
    """Return user fields as a dict."""
    return {
        "name": user.name,
        "email": user.email,
        "role": user.role,
    }

def _build_metadata_fields(metadata: Metadata) -> dict[str, Any]:
    """Return metadata fields as a dict."""
    return {
        "created_at": metadata.created_at,
        "version": metadata.version,
    }

def build_response(user: User, metadata: Metadata) -> dict[str, Any]:
    return {
        **_build_user_fields(user),
        **_build_metadata_fields(metadata),
    }

Refactoring Patterns

Pattern 1: Extract Computation

When a function has complex logic that computes a value, extract it.

# BEFORE - complex inline computation
def process_order(order: Order) -> Summary:
    # ... lots of code ...
    total = 0
    for item in order.items:
        price = item.unit_price * item.quantity
        if item.discount:
            price *= (1 - item.discount)
        if order.member:
            price *= 0.9
        total += price
    # ... more code ...

# AFTER - extracted pure helper
def _calculate_item_price(item: OrderItem, is_member: bool) -> Decimal:
    """Calculate final price for a single item."""
    price = item.unit_price * item.quantity
    if item.discount:
        price *= (1 - item.discount)
    if is_member:
        price *= Decimal("0.9")
    return price

def _calculate_order_total(order: Order) -> Decimal:
    """Calculate total price for all items."""
    return sum(
        _calculate_item_price(item, order.member)
        for item in order.items
    )

def process_order(order: Order) -> Summary:
    # ... lots of code ...
    total = _calculate_order_total(order)
    # ... more code ...

Pattern 2: Extract Collection Building

When building a list or dict, extract the transformation logic.

# INCORRECT - void mutator for list building
def _add_valid_items(results: list[Item], items: list[Item]) -> None:
    for item in items:
        if item.is_valid and item.score > 0.5:
            results.append(item)

# CORRECT - return the filtered list
def _filter_valid_items(items: list[Item]) -> list[Item]:
    """Return items that are valid with score > 0.5."""
    return [item for item in items if item.is_valid and item.score > 0.5]

# Even better - use comprehension inline if simple enough
valid_items = [item for item in items if item.is_valid and item.score > 0.5]

Pattern 3: Extract Conditional Logic

When a function has complex branching, extract the decision or transformation.

# BEFORE - complex conditional
def format_value(value: Any, config: Config) -> str:
    if isinstance(value, datetime):
        if config.use_iso:
            return value.isoformat()
        else:
            return value.strftime(config.date_format)
    elif isinstance(value, Decimal):
        if config.currency:
            return f"${value:.2f}"
        else:
            return str(value)
    # ... more cases ...

# AFTER - extracted formatters
def _format_datetime(value: datetime, config: Config) -> str:
    """Format datetime according to config."""
    if config.use_iso:
        return value.isoformat()
    return value.strftime(config.date_format)

def _format_decimal(value: Decimal, config: Config) -> str:
    """Format decimal according to config."""
    if config.currency:
        return f"${value:.2f}"
    return str(value)

def format_value(value: Any, config: Config) -> str:
    if isinstance(value, datetime):
        return _format_datetime(value, config)
    if isinstance(value, Decimal):
        return _format_decimal(value, config)
    # ... more cases ...

Pattern 4: Extract Validation

When validation logic is complex, extract validators that return results.

# INCORRECT - validation that mutates error list
def _validate_email(user: User, errors: list[str]) -> None:
    if not user.email:
        errors.append("Email is required")
    elif "@" not in user.email:
        errors.append("Invalid email format")

# CORRECT - validation that returns errors
def _validate_email(email: str | None) -> list[str]:
    """Return list of validation errors for email (empty if valid)."""
    if not email:
        return ["Email is required"]
    if "@" not in email:
        return ["Invalid email format"]
    return []

def validate_user(user: User) -> list[str]:
    """Return all validation errors for user."""
    return [
        *_validate_email(user.email),
        *_validate_name(user.name),
        *_validate_role(user.role),
    ]

Checklist When Extracting Helpers

Before creating a helper function during refactoring, verify:

• Does it return a value? (Not

  None

  with side effects)
• Does it avoid mutating any input parameters?
• Can it be tested in isolation without setup?
• Does it have a single responsibility?
• Is the data flow visible at the call site?

If any answer is "no", redesign the helper.

Validation

Check complexity after refactoring:

ruff check --select C901 <file>

The refactored code should have no functions exceeding McCabe complexity of 5.