You open a function to fix a one-line bug and forty minutes later you’re still tracing which branch sets which variable. Three unrelated jobs are welded into one block, and none of them has a name. Pull one thread and you can’t tell which others move.

That is spaghetti code, and the name has nothing to do with indentation or naming style. It describes control flow: execution jumps between conditions the way one strand of pasta crosses every other strand in the bowl, and following a single path to its end means holding four levels of if in your head at once.

What actually makes code spaghetti code

Four symptoms show up together in nearly every case, and each one is fixable on its own:

  • Deep nesting. Three or four levels of if/else inside each other. Each level adds indentation and a condition you have to keep true in your head to know which line you’re reading.
  • Functions doing several jobs. One function validates input, computes a result, mutates state and logs a message. Change the computation and you risk the logging; test the validation and you have to fake the state.
  • Hidden shared state. A global variable or a module-level dict, mutated from inside a function whose name gives no hint of it. Two callers can step on each other and neither one’s code shows why.
  • Copy-paste branches. The same shape of logic repeated with one value changed, instead of extracted once. Fix one copy, forget the other, and you have a bug that only shows up on half the inputs.

None of these needs a rewrite. Each has a named refactor that removes it without changing what the code does. That constraint matters more than it sounds: if you spot a bug while cleaning up, don’t fix it in the same change. Do it separately, or you can’t tell which change caused which regression.

A real function with every spaghetti code symptom

Here’s process_order, from a small store’s checkout flow. Read it once and note where you lose track.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35

INVENTORY = {"usb-cable": 42, "laptop-stand": 7, "webcam": 0}

def process_order(order, is_vip):
    total = 0
    if order["items"]:
        for item in order["items"]:
            if item["sku"] in INVENTORY:
                if INVENTORY[item["sku"]] >= item["qty"]:
                    price = item["price"]
                    if is_vip:
                        if item["qty"] > 10:
                            price = price * 0.8
                        else:
                            price = price * 0.9
                    else:
                        if item["qty"] > 10:
                            price = price * 0.95
                        else:
                            price = price
                    total = total + price * item["qty"]
                    INVENTORY[item["sku"]] -= item["qty"]
                    print(f"shipped {item['qty']} x {item['sku']}")
                else:
                    print(f"not enough stock for {item['sku']}")
            else:
                print(f"unknown sku {item['sku']}")
    else:
        print("empty order")

    if total > 100:
        shipping = 0 if order["country"] == "US" else 15
    else:
        shipping = 5 if order["country"] == "US" else 20

    return total + shipping

All four symptoms are here. The line that charges the customer sits four levels deep. The function checks stock, prices items, applies a loyalty discount, mutates INVENTORY, ships (via print, standing in for a real shipping call) and totals the order. INVENTORY is a module-level dict any other function can also mutate, and nothing in the signature warns you. The VIP and non-VIP branches are the same shape with different numbers, so a third discount tier means a third copy.

How to refactor spaghetti code without breaking it

One small, mechanical change at a time. Each one leaves behavior identical and gets its own commit. If step 3 breaks something, git bisect can find which of five small commits did it. A single “refactor everything” commit gives bisect nothing to narrow.

Pin the behavior with a characterization test

Before changing a line, write a test that records what the function does today. Not what it should do. This is a characterization test: you’re freezing current behavior so you notice the moment a “safe” change isn’t.

1
2
3
4
5
6
7
8
9

def test_current_behavior_vip_bulk_order():
    INVENTORY["usb-cable"] = 42
    order = {"items": [{"sku": "usb-cable", "qty": 12, "price": 10.0}], "country": "US"}
    assert process_order(order, is_vip=True) == 101.0  # 12 * 10 * 0.8 = 96, under the $100 free-shipping cutoff, + $5 US shipping
    assert INVENTORY["usb-cable"] == 30

def test_current_behavior_unknown_sku():
    order = {"items": [{"sku": "flux-capacitor", "qty": 1, "price": 999}], "country": "DE"}
    assert process_order(order, is_vip=False) == 20  # nothing shipped, DE shipping under $100

Run both against the untouched function first, so you know the expected numbers come from the code and not from a typo. Reset INVENTORY before each test. Every step below runs against this same pair. While they pass, the behavior hasn’t moved.

Extract functions for pricing and shipping

Pull the pricing and shipping logic into functions whose names say what they do. The logic itself doesn’t change in this step.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29

def price_line_item(item, is_vip):
    price = item["price"]
    if is_vip:
        price *= 0.8 if item["qty"] > 10 else 0.9
    else:
        price *= 0.95 if item["qty"] > 10 else 1.0
    return price * item["qty"]

def calculate_shipping(total, country):
    if total > 100:
        return 0 if country == "US" else 15
    return 5 if country == "US" else 20

def process_order(order, is_vip):
    total = 0
    if order["items"]:
        for item in order["items"]:
            if item["sku"] in INVENTORY:
                if INVENTORY[item["sku"]] >= item["qty"]:
                    total += price_line_item(item, is_vip)
                    INVENTORY[item["sku"]] -= item["qty"]
                    print(f"shipped {item['qty']} x {item['sku']}")
                else:
                    print(f"not enough stock for {item['sku']}")
            else:
                print(f"unknown sku {item['sku']}")
    else:
        print("empty order")
    return total + calculate_shipping(total, order["country"])

Run the tests: same two numbers, same inventory count. Commit with a message that says exactly what happened: “extract price_line_item and calculate_shipping, no behavior change”.

Replace nested conditionals with guard clauses

The stock check in process_order is three levels of nesting around the one line that matters. Turn each check into an early exit for the case that can’t proceed, and the success path stops being indented under everything else.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18

def process_order(order, is_vip):
    if not order["items"]:
        print("empty order")
        return 0 + calculate_shipping(0, order["country"])

    total = 0
    for item in order["items"]:
        if item["sku"] not in INVENTORY:
            print(f"unknown sku {item['sku']}")
            continue
        if INVENTORY[item["sku"]] < item["qty"]:
            print(f"not enough stock for {item['sku']}")
            continue
        total += price_line_item(item, is_vip)
        INVENTORY[item["sku"]] -= item["qty"]
        print(f"shipped {item['qty']} x {item['sku']}")

    return total + calculate_shipping(total, order["country"])

Same tests, same results. The loop body now reads top to bottom: reject, reject, do the work. These are the same branches as before, one level deep instead of three.

Replace the flag argument with a lookup table

is_vip switches what price_line_item does internally. Every call site has to know the boolean exists, and a third tier means a third copy-pasted branch. Move the numbers into data:

1
2
3
4
5
6
7
8
9

DISCOUNT_RATES = {
    "vip": {"bulk": 0.8, "standard": 0.9},
    "regular": {"bulk": 0.95, "standard": 1.0},
}

def price_line_item(item, tier):
    rates = DISCOUNT_RATES[tier]
    rate = rates["bulk"] if item["qty"] > 10 else rates["standard"]
    return item["price"] * rate * item["qty"]

process_order changes the same way: its parameter is renamed from is_vip to tier, and the one call it makes to price_line_item(item, is_vip) becomes price_line_item(item, tier). Nothing else in the function’s body changes. A call like process_order(order, tier="vip") reads on its own, and a new tier is one new entry in DISCOUNT_RATES instead of another if/else arm. This is the one step that changes a signature, so the tests need a matching edit: pass tier="vip" instead of is_vip=True. The assertions stay as they are, because the output for the same order hasn’t changed.

Pull side effects to the edges

price_line_item is now pure: same inputs, same output, nothing mutated. process_order still mutates INVENTORY and calls print in the middle of a loop, which is why testing it needs a global reset instead of a plain function call. Separate deciding from doing. Compute what should happen, return it, and let the caller act on it.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33

def plan_order(order, tier):
    if not order["items"]:
        return {"total": calculate_shipping(0, order["country"]), "shipments": [], "messages": ["empty order"]}

    remaining = dict(INVENTORY)
    total = 0
    shipments, messages = [], []
    for item in order["items"]:
        sku = item["sku"]
        if sku not in remaining:
            messages.append(f"unknown sku {sku}")
            continue
        if remaining[sku] < item["qty"]:
            messages.append(f"not enough stock for {sku}")
            continue
        total += price_line_item(item, tier)
        remaining[sku] -= item["qty"]
        shipments.append((sku, item["qty"]))
        messages.append(f"shipped {item['qty']} x {sku}")

    return {
        "total": total + calculate_shipping(total, order["country"]),
        "shipments": shipments,
        "messages": messages,
    }

def process_order(order, tier):
    plan = plan_order(order, tier)
    for sku, qty in plan["shipments"]:
        INVENTORY[sku] -= qty
    for message in plan["messages"]:
        print(message)
    return plan["total"]

Two details keep this an exact match for the original, not just a close one. The empty-order branch still routes through calculate_shipping instead of returning a flat 0 — the original charges shipping even on an empty order, so the refactor has to keep charging it too. And plan_order checks stock against a local remaining copy it decrements as it goes, not against INVENTORY directly, so an order with the same SKU on two lines still gets checked against decreasing stock exactly like the original’s inline decrement did — check both lines against the same untouched number and a second line can pass a check the first line should have used up.

plan_order reads INVENTORY but never writes to it or prints, so you can call it a thousand times in a test without resetting anything. process_order is now the only place that touches INVENTORY or print, and it fits on one screen with room to spare.

What the refactored function gains over the original

Nesting went from four levels to one. Each function does the one job its name describes. INVENTORY is written from exactly one place, and there’s no copy-pasted branch left to fix twice. Adding a “wholesale” tier is a one-line change to DISCOUNT_RATES.

The history matters as much as the code. You now have five commits (the test, then four refactors), each green against the same characterization tests. If a discount comes out wrong six months from now, git blame on the pricing line lands on a commit small enough to read in full. Squash all five into one “refactor” commit when the branch merges and you lose that. Git Rebase vs Merge covers how each merge strategy shapes the history you’re left with.

When not to refactor spaghetti code

A characterization test needs the current behavior to be observable and worth preserving. If the deadline is today and there’s no test to lean on, refactoring turns a working mess into a broken mess with better formatting. Ship the fix, then schedule the cleanup as its own task. Don’t smuggle it into a bug-fix commit where it hides the actual change.

Code scheduled for deletion doesn’t earn a refactor either. If the feature is being replaced next sprint, the hours go into the replacement, not into polishing the old version for the two weeks it has left.

Restructuring code with no test at all is a rewrite, and you have no way to know what it broke. If you can’t test the current behavior, the first step is making it testable.

Pick one function you avoid opening, write a single characterization test against what it does right now, and make the first extraction. Commit it before you touch anything else.