Refactoring a 120-Line Function Into Something You'd Actually Want to Read

The function below started small. It processed an order: check the items, apply a discount, calculate tax, save it, email the customer. Over about a year of "just add this one thing" commits, it grew to roughly 120 lines, four levels of nested conditionals deep, and became the function nobody wanted to touch. What follows is a condensed but faithful version of it, and the actual steps I took to turn it into something a new team member could understand in under a minute.

The Function, As It Actually Looked

Here's the core of it. I've trimmed some repeated boilerplate for length, but the structure — the nesting, the magic numbers, the mixed responsibilities — is exactly how it shipped:

function processOrder($order, $user, $items) {
    $total = 0;
    $discount = 0;
    $errors = [];

    if ($order != null) {
        if ($user != null) {
            if (count($items) > 0) {
                foreach ($items as $item) {
                    if ($item['quantity'] > 0) {
                        $total += $item['price'] * $item['quantity'];
                    } else {
                        $errors[] = 'Invalid quantity for ' . $item['name'];
                    }
                }

                if ($total > 100) {
                    if ($user['is_member']) {
                        $discount = $total * 0.15;
                    } else {
                        $discount = $total * 0.05;
                    }
                } else {
                    if ($user['is_member']) {
                        $discount = $total * 0.1;
                    }
                }

                if ($user['country'] == 'US') {
                    $tax = ($total - $discount) * 0.0825;
                } else if ($user['country'] == 'CA') {
                    $tax = ($total - $discount) * 0.13;
                } else {
                    $tax = 0;
                }

                $finalTotal = $total - $discount + $tax;

                if (empty($errors)) {
                    $order['total'] = $finalTotal;
                    $order['status'] = 'confirmed';
                    saveOrder($order);
                    mail($user['email'], 'Order Confirmed', 'Your total is ' . $finalTotal);
                    error_log('Order ' . $order['id'] . ' processed for ' . $user['email']);
                    return ['success' => true, 'total' => $finalTotal];
                } else {
                    return ['success' => false, 'errors' => $errors];
                }
            } else {
                $errors[] = 'No items in order';
                return ['success' => false, 'errors' => $errors];
            }
        } else {
            $errors[] = 'No user';
            return ['success' => false, 'errors' => $errors];
        }
    } else {
        $errors[] = 'No order';
        return ['success' => false, 'errors' => $errors];
    }
}

Nothing in this function is individually wrong. Every line does something reasonable. The problem is that it does four unrelated jobs — validation, pricing math, persistence, and notifications — all interleaved in one scope, at four levels of indentation, with no way to test the discount logic without also triggering a real email send.

Step 1: Flatten the Nesting With Guard Clauses

The outer three if statements (order exists, user exists, items exist) are all doing the same kind of thing: bailing out early if a precondition fails. Nesting them means every line of actual logic sits four indents deep before you've written anything meaningful. Guard clauses fix this without changing behavior at all — they just invert the conditions and return immediately instead of wrapping the rest of the function in an else.

This step alone, before extracting anything else, made the function's real logic sit at one level of indentation instead of four. That's not cosmetic — deep nesting is one of the strongest predictors of "I can't hold this whole function in my head at once."

Step 2: Pull Validation Into Its Own Function

Validation and business logic don't need to live in the same function just because they run in sequence. Separating them means you can test "does this reject an order with a zero quantity" without also running the discount and tax math.

function validateOrder(?array $order, ?array $user, array $items): array
{
    $errors = [];

    if ($order === null) {
        $errors[] = 'No order';
    }
    if ($user === null) {
        $errors[] = 'No user';
    }
    if (count($items) === 0) {
        $errors[] = 'No items in order';
    }

    foreach ($items as $item) {
        if ($item['quantity'] <= 0) {
            $errors[] = 'Invalid quantity for ' . $item['name'];
        }
    }

    return $errors;
}

Notice this function has exactly one job: look at inputs, return a list of problems. It doesn't calculate anything, doesn't save anything, doesn't send anything. That single-responsibility boundary is what makes it trivial to test in isolation later.

Step 3: Replace Magic Numbers With Named Discount Logic

The original had 0.15, 0.05, 0.1, and 100 scattered through nested conditionals with no explanation of what any of them meant. Six months later, nobody on the team could confidently say whether 100 was a dollar threshold or something else without reading the surrounding logic carefully. Extracting this into its own function with clear parameter names fixes that:

function calculateDiscount(float $subtotal, array $user): float
{
    $isLargeOrder = $subtotal > 100;

    if ($user['is_member']) {
        return $subtotal * ($isLargeOrder ? 0.15 : 0.10);
    }

    return $isLargeOrder ? $subtotal * 0.05 : 0.0;
}

This isn't just shorter than the nested version — it's a function you can call with four different combinations of inputs and immediately see whether the output is right, without any of the surrounding order-processing noise. That's the actual point of the extraction: not brevity, testability.

Step 4: Do the Same for Tax

The tax logic had the same shape as the discount logic — a chain of conditionals keyed off a field on $user. Extracting it makes the rate table explicit instead of buried in if/else if branches:

function calculateTax(float $taxableAmount, string $country): float
{
    $rates = ['US' => 0.0825, 'CA' => 0.13];

    return $taxableAmount * ($rates[$country] ?? 0.0);
}

Written as a lookup table, adding a third country later is a one-line change to the $rates array instead of another else if branch grafted onto a growing chain. That's a small thing on paper, but it's exactly the kind of change that used to require re-reading the whole nested block to know where the new branch safely goes.

Step 5: Separate Side Effects From Calculation

The original function calculated the total and then, in the same breath, saved to the database, sent an email, and wrote a log line. Mixing pure calculation with side effects is what made this function impossible to unit test without a database connection and a working mail server. Pulling the side effects into their own small functions isn't about reducing line count — it's about making the "what does this number turn out to be" question answerable without also triggering real I/O.

function notifyCustomer(array $user, float $total): void
{
    mail($user['email'], 'Order Confirmed', 'Your total is ' . number_format($total, 2));
}

function logOrder(array $order, array $user): void
{
    error_log('Order ' . $order['id'] . ' processed for ' . $user['email']);
}

The Result: Composing the Pieces

With validation, pricing, and side effects each living in their own function, the top-level function shrinks down to something that reads almost like a checklist of what happens, in order:

function processOrder(?array $order, ?array $user, array $items): array
{
    $errors = validateOrder($order, $user, $items);
    if (! empty($errors)) {
        return ['success' => false, 'errors' => $errors];
    }

    $subtotal = calculateSubtotal($items);
    $discount = calculateDiscount($subtotal, $user);
    $tax = calculateTax($subtotal - $discount, $user['country']);
    $total = $subtotal - $discount + $tax;

    $order['total'] = $total;
    $order['status'] = 'confirmed';
    saveOrder($order);
    notifyCustomer($user, $total);
    logOrder($order, $user);

    return ['success' => true, 'total' => $total];
}

This is roughly 20 lines instead of 120, but the line count isn't the win — plenty of bad code is also short. The win is that every function here answers exactly one question, none of them are nested more than one level deep, and six of the seven can be tested without touching a database or sending a real email.

Knowing When to Stop Extracting

It's worth saying explicitly, because it's the mistake people make right after learning this technique: extraction is not free, and it's possible to overdo it. If I'd also pulled $subtotal - $discount + $tax into a one-line function called calculateFinalTotal, I'd have added an indirection that doesn't earn its keep — anyone reading processOrder would have to jump to another function just to see basic arithmetic that was perfectly readable inline.

The line I use to decide: extract when a piece of logic has its own reason to change independently of the rest, or when it's complex enough that naming it clarifies intent, or when it's something you'd want to test in isolation. Simple arithmetic that only ever appears in one place and reads clearly on its own usually fails all three of those tests, and extracting it anyway just adds a layer of navigation between a reader and the thing they're trying to understand.

The same caution applies to the validation function. It would be tempting to keep splitting validateOrder into validateOrderExists, validateUserExists, and validateItemQuantities separately. I tried that during this refactor and reverted it — the three checks are so tightly related, and so cheap to read together, that splitting them further made the caller do more work assembling the results than it saved in readability. Refactoring has a stopping point, and it's usually the moment where the next split would cost more in indirection than it returns in clarity.

How This Reads in Code Review

The real test of a refactor isn't whether it compiles — it's whether a teammate who has never seen this code can review the new version faster than they could have reviewed the old one. When I put the "after" version up for review, the discussion was entirely about the discount percentages and the tax rates — actual business logic — instead of about untangling which branch of which nested condition a given line belonged to. That shift, from reviewing structure to reviewing intent, is the actual payoff of this kind of cleanup, and it's a good signal to watch for in your own refactors: if reviewers are still asking "wait, which condition does this run under," the extraction isn't finished yet.

What Changed, Concretely

• Four levels of nesting became one, via guard clauses at the top
• Validation, pricing, persistence, and notification became four separate, independently callable functions
• Magic numbers moved into named, commentable locations instead of sitting bare inside conditionals
• The tax "if chain" became a lookup table that's a one-line change to extend
• Pure calculation (discount, tax, subtotal) is now fully separated from side effects (save, email, log)

The same principle scales down as well as up — even a small tool benefits from this separation. Our word counter tool keeps the actual counting logic as a pure function, completely separate from the code that formats the result and the code that updates the page, for exactly this reason: each piece can change independently without the others needing to know about it.

Where to Go From Here

A refactor like this isn't complete until it's backed by tests — otherwise you're trusting that the behavior stayed identical purely by careful reading, which is exactly the kind of manual verification that got this function into a mess in the first place. The functions extracted here, especially calculateDiscount and calculateTax, are close to ideal first candidates for testing because they're pure: same inputs, same output, no side effects to mock. If you haven't written tests before, our guide on writing your first automated test suite for a small PHP app picks up exactly where this refactor leaves off.

The lesson that generalizes beyond this one function: when you catch yourself nesting a fourth if inside a function, that's usually not a sign you need a cleverer conditional. It's a sign the function is doing more than one job, and the fix is almost always to give each job its own name.