Sie öffnen eine Funktion, um einen Ein-Zeilen-Bug zu beheben, und vierzig Minuten spĂ€ter verfolgen Sie immer noch, welcher Zweig welche Variable setzt. Drei voneinander unabhĂ€ngige Aufgaben sind in einem einzigen Block zusammengeschweißt, und keine von ihnen hat einen Namen. Sie ziehen an einem Faden und können nicht sagen, welche anderen sich mitbewegen.

Das ist Spaghetti-Code, und der Name hat nichts mit EinrĂŒckung oder Namensstil zu tun. Er beschreibt den Kontrollfluss: Die AusfĂŒhrung springt zwischen Bedingungen hin und her, so wie ein Nudelstrang jeden anderen in der SchĂŒssel kreuzt, und einem einzigen Pfad bis zum Ende zu folgen bedeutet, vier Ebenen von if gleichzeitig im Kopf zu behalten.

Was Code tatsÀchlich zu Spaghetti-Code macht

Vier Symptome treten fast immer gemeinsam auf, und jedes lÀsst sich einzeln beheben:

  • Tiefe Verschachtelung. Drei oder vier Ebenen von if/else ineinander verschachtelt. Jede Ebene fĂŒgt EinrĂŒckung und eine Bedingung hinzu, die Sie im Kopf als wahr behalten mĂŒssen, um zu wissen, welche Zeile Sie gerade lesen.
  • Funktionen, die mehrere Aufgaben erledigen. Eine Funktion validiert die Eingabe, berechnet ein Ergebnis, verĂ€ndert den Zustand und schreibt eine Log-Nachricht. Sie Ă€ndern die Berechnung und riskieren das Logging; Sie testen die Validierung und mĂŒssen den Zustand simulieren.
  • Versteckter gemeinsamer Zustand. Eine globale Variable oder ein Dictionary auf Modulebene, verĂ€ndert aus dem Inneren einer Funktion, deren Name keinen Hinweis darauf gibt. Zwei Aufrufer können sich gegenseitig ins Gehege kommen, ohne dass der Code von beiden zeigt, warum.
  • Copy-Paste-Zweige. Die gleiche Form von Logik wiederholt sich mit einem geĂ€nderten Wert, statt einmal extrahiert zu werden. Sie korrigieren eine Kopie, vergessen die andere, und haben einen Bug, der nur bei der HĂ€lfte der Eingaben auftritt.

Keines davon erfordert eine Neuschreibung. Jedes hat ein benanntes Refactoring, das es beseitigt, ohne zu Ă€ndern, was der Code tut. Diese EinschrĂ€nkung zĂ€hlt mehr, als sie klingt: Wenn Sie beim AufrĂ€umen einen Bug entdecken, beheben Sie ihn nicht in derselben Änderung. Machen Sie es getrennt, sonst können Sie nicht sagen, welche Änderung welche Regression verursacht hat.

Eine echte Funktion mit jedem Spaghetti-Code-Symptom

Hier ist process_order aus dem Checkout-Ablauf eines kleinen Shops. Lesen Sie sie einmal durch und achten Sie darauf, wo Sie den Faden verlieren.

 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

Alle vier Symptome sind hier vorhanden. Die Zeile, die den Kunden belastet, liegt vier Ebenen tief. Die Funktion prĂŒft den Lagerbestand, berechnet Preise, wendet einen Treuerabatt an, verĂ€ndert INVENTORY, versendet (ĂŒber print, als Platzhalter fĂŒr einen echten Versandaufruf) und summiert die Bestellung. INVENTORY ist ein Dictionary auf Modulebene, das auch jede andere Funktion verĂ€ndern kann, und nichts in der Signatur warnt Sie davor. Die VIP- und Nicht-VIP-Zweige haben dieselbe Form mit unterschiedlichen Zahlen, sodass eine dritte Rabattstufe eine dritte Kopie bedeutet.

Wie man Spaghetti-Code refactort, ohne ihn zu zerstören

Eine kleine, mechanische Änderung nach der anderen. Jede lĂ€sst das Verhalten unverĂ€ndert und bekommt ihren eigenen Commit. Wenn Schritt 3 etwas kaputt macht, kann git bisect herausfinden, welcher der fĂŒnf kleinen Commits es war. Ein einziger „alles refactoren“-Commit gibt bisect nichts, womit es eingrenzen kann.

Verhalten mit einem Charakterisierungstest festhalten

Bevor Sie eine Zeile Ă€ndern, schreiben Sie einen Test, der festhĂ€lt, was die Funktion heute tut. Nicht, was sie tun sollte. Das ist ein Charakterisierungstest: Sie frieren das aktuelle Verhalten ein, um den Moment zu bemerken, in dem eine „sichere“ Änderung es nicht ist.

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, unter der 100-$-Grenze fĂŒr kostenlosen Versand, + $5 US-Versand
    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  # nichts versendet, DE-Versand unter $100

FĂŒhren Sie beide zuerst gegen die noch unverĂ€nderte Funktion aus, damit Sie wissen, dass die erwarteten Zahlen aus dem Code stammen und nicht aus einem Tippfehler. Setzen Sie INVENTORY vor jedem Test zurĂŒck. Jeder Schritt unten lĂ€uft gegen dasselbe Paar. Solange sie bestehen, hat sich das Verhalten nicht verschoben.

Funktionen fĂŒr Preis und Versand extrahieren

Ziehen Sie die Preis- und Versandlogik in Funktionen, deren Namen sagen, was sie tun. Die Logik selbst Àndert sich in diesem Schritt nicht.

 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"])

FĂŒhren Sie die Tests aus: dieselben zwei Zahlen, derselbe Lagerbestand. Committen Sie mit einer Nachricht, die genau sagt, was passiert ist: „extract price_line_item and calculate_shipping, no behavior change“.

Verschachtelte Bedingungen durch Guard Clauses ersetzen

Die BestandsprĂŒfung in process_order besteht aus drei Verschachtelungsebenen rund um die eine Zeile, die zĂ€hlt. Verwandeln Sie jede PrĂŒfung in einen frĂŒhen Ausstieg fĂŒr den Fall, der nicht fortfahren kann. Der Erfolgspfad steht danach nicht mehr unter allem anderen eingerĂŒckt.

 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"])

Gleiche Tests, gleiche Ergebnisse. Der Schleifenkörper liest sich jetzt von oben nach unten: ablehnen, ablehnen, die Arbeit erledigen. Es sind dieselben Zweige wie zuvor, nur eine statt drei Ebenen tief.

Das Flag-Argument durch eine Lookup-Tabelle ersetzen

is_vip schaltet um, was price_line_item intern tut. Jede Aufrufstelle muss wissen, dass der boolesche Wert existiert, und eine dritte Stufe bedeutet einen dritten kopierten Zweig. Verschieben Sie die Zahlen in Daten:

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 Ă€ndert sich auf dieselbe Weise: ihr Parameter wird von is_vip in tier umbenannt, und der eine Aufruf von price_line_item(item, is_vip) wird zu price_line_item(item, tier). Sonst Ă€ndert sich nichts im Funktionskörper. Ein Aufruf wie process_order(order, tier="vip") liest sich von selbst, und eine neue Stufe ist ein neuer Eintrag in DISCOUNT_RATES statt eines weiteren if/else-Zweigs. Das ist der einzige Schritt, der eine Signatur Ă€ndert, also brauchen die Tests eine entsprechende Anpassung: Übergeben Sie tier="vip" statt is_vip=True. Die Assertions bleiben, wie sie sind, weil sich die Ausgabe fĂŒr dieselbe Bestellung nicht geĂ€ndert hat.

Seiteneffekte an die RĂ€nder verschieben

price_line_item ist jetzt rein: gleiche Eingaben, gleiche Ausgabe, nichts wird verĂ€ndert. process_order verĂ€ndert weiterhin INVENTORY und ruft mitten in einer Schleife print auf, weshalb das Testen einen globalen Reset statt eines einfachen Funktionsaufrufs braucht. Trennen Sie das Entscheiden vom Tun. Berechnen Sie, was passieren soll, geben Sie es zurĂŒck, und lassen Sie den Aufrufer entsprechend handeln.

 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"]

Zwei Details sorgen dafĂŒr, dass dies eine exakte Übereinstimmung mit dem Original ist, nicht nur eine ungefĂ€hre.

Der Zweig fĂŒr die leere Bestellung lĂ€uft weiterhin ĂŒber calculate_shipping, statt eine feste 0 zurĂŒckzugeben. Das Original berechnet Versandkosten sogar fĂŒr eine leere Bestellung, also muss das Refactoring das ebenfalls tun.

Und plan_order prĂŒft den Bestand gegen eine lokale Kopie remaining, die es nach und nach verringert, nicht direkt gegen INVENTORY. So wird eine Bestellung mit demselben SKU auf zwei Zeilen weiterhin gegen einen sinkenden Bestand geprĂŒft, genau wie es die Inline-Verringerung des Originals tat. Beide Zeilen gegen dieselbe unverĂ€nderte Zahl zu prĂŒfen, wĂŒrde einer zweiten Zeile erlauben, eine PrĂŒfung zu bestehen, die die erste bereits hĂ€tte aufbrauchen sollen.

plan_order liest INVENTORY, schreibt aber nie hinein und gibt nichts aus, sodass Sie sie tausendmal in einem Test aufrufen können, ohne irgendetwas zurĂŒckzusetzen. process_order ist jetzt die einzige Stelle, die INVENTORY oder print berĂŒhrt, und passt mit Platz ĂŒbrig auf einen Bildschirm.

Was die refactorte Funktion gegenĂŒber dem Original gewinnt

Die Verschachtelung ist von vier Ebenen auf eine gesunken. Jede Funktion erledigt die eine Aufgabe, die ihr Name beschreibt. INVENTORY wird von genau einer Stelle aus geschrieben, und es bleibt kein kopierter Zweig ĂŒbrig, den man zweimal korrigieren mĂŒsste. Eine „Großhandel“-Stufe hinzuzufĂŒgen ist eine Ein-Zeilen-Änderung in DISCOUNT_RATES.

Die Historie zĂ€hlt genauso wie der Code. Sie haben jetzt fĂŒnf Commits (den Test, dann vier Refactorings), jeder grĂŒn gegen dieselben Charakterisierungstests. Wenn in sechs Monaten ein Rabatt falsch herauskommt, landet git blame auf der Preiszeile bei einem Commit, der klein genug ist, um ihn vollstĂ€ndig zu lesen. Quetschen Sie alle fĂŒnf beim Merge des Branches zu einem einzigen „refactor“-Commit zusammen, und Sie verlieren genau das. Git Rebase vs Merge erklĂ€rt, wie jede Merge-Strategie die Historie formt, die Ihnen bleibt.

Wann man Spaghetti-Code nicht refactoren sollte

Ein Charakterisierungstest braucht ein aktuelles Verhalten, das beobachtbar ist und es wert ist, erhalten zu werden. Wenn die Deadline heute ist und es keinen Test gibt, auf den Sie sich stĂŒtzen können, macht Refactoring aus einem funktionierenden Chaos ein kaputtes Chaos mit besserer Formatierung. Liefern Sie den Fix aus, und planen Sie das AufrĂ€umen dann als eigene Aufgabe. Schmuggeln Sie es nicht in einen Bugfix-Commit, wo es die eigentliche Änderung verdeckt.

Auch Code, der zur Löschung ansteht, verdient kein Refactoring. Wenn das Feature im nĂ€chsten Sprint ersetzt wird, fließen die Stunden in den Ersatz, nicht ins Polieren der alten Version fĂŒr die zwei Wochen, die ihr noch bleiben.

Code ohne jeden Test umzustrukturieren ist eine Neuschreibung, und Sie haben keine Möglichkeit zu wissen, was Sie kaputt gemacht haben. Wenn Sie das aktuelle Verhalten nicht testen können, besteht der erste Schritt darin, es testbar zu machen.

WĂ€hlen Sie eine Funktion aus, um die Sie sonst einen Bogen machen, schreiben Sie einen einzigen Charakterisierungstest gegen das, was sie gerade tut, und nehmen Sie die erste Extraktion vor. Committen Sie, bevor Sie irgendetwas anderes anfassen.

Verwandte Artikel