Apri una funzione per sistemare un bug di una riga e quaranta minuti dopo stai ancora seguendo quale ramo imposta quale variabile. Tre compiti che non c’entrano nulla tra loro sono saldati in un unico blocco, e nessuno dei tre ha un nome. Tiri un filo e non riesci a capire quali altri si muovono.

Questo è lo spaghetti code, e il nome non c’entra niente con l’indentazione o lo stile dei nomi. Descrive il flusso di controllo: l’esecuzione salta tra le condizioni come un filo di pasta che attraversa tutti gli altri nella ciotola, e seguire un singolo percorso fino alla fine significa tenere in testa quattro livelli di if contemporaneamente.

Cosa rende un codice davvero spaghetti code

Quattro sintomi compaiono insieme in quasi tutti i casi, e ognuno si può correggere da solo:

  • Nidificazione profonda. Tre o quattro livelli di if/else uno dentro l’altro. Ogni livello aggiunge indentazione e una condizione che devi tenere vera in testa per capire quale riga stai leggendo.
  • Funzioni che fanno più lavori insieme. Una funzione valida l’input, calcola un risultato, modifica lo stato e scrive un log. Cambi il calcolo e rischi di rompere il log; testi la validazione e devi simulare lo stato.
  • Stato condiviso nascosto. Una variabile globale o un dizionario a livello di modulo, modificato dall’interno di una funzione il cui nome non lo lascia intuire. Due chiamanti possono pestarsi i piedi a vicenda e il codice di nessuno dei due mostra perché.
  • Rami copia-incolla. La stessa forma di logica ripetuta cambiando un solo valore, invece di estrarla una volta sola. Correggi una copia, ti dimentichi l’altra, e hai un bug che si manifesta solo su metà degli input.

Nessuno di questi richiede una riscrittura. Ognuno ha un refactoring con un nome preciso che lo elimina senza cambiare cosa fa il codice. Questo vincolo conta più di quanto sembri: se noti un bug mentre stai pulendo, non correggerlo nella stessa modifica. Fallo separatamente, altrimenti non riesci a capire quale cambiamento ha causato quale regressione.

Una funzione reale con tutti i sintomi dello spaghetti code

Ecco process_order, dal flusso di checkout di un piccolo negozio. Leggila una volta e nota dove perdi il filo.

 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

Ci sono tutti e quattro i sintomi. La riga che addebita il cliente sta quattro livelli sotto. La funzione controlla lo stock, calcola i prezzi, applica uno sconto fedeltà, modifica INVENTORY, spedisce (tramite print, al posto di una vera chiamata di spedizione) e totalizza l’ordine. INVENTORY è un dizionario a livello di modulo che qualsiasi altra funzione può modificare, e niente nella firma te lo segnala. I rami VIP e non-VIP hanno la stessa forma con numeri diversi, quindi un terzo livello di sconto significa una terza copia.

Come rifattorizzare lo spaghetti code senza romperlo

Una modifica piccola e meccanica alla volta. Ognuna lascia il comportamento identico e ha il suo commit. Se il passo 3 rompe qualcosa, git bisect può trovare quale dei cinque piccoli commit è stato. Un unico commit “rifattorizza tutto” non dà a bisect niente da restringere.

Blocca il comportamento con un test di caratterizzazione

Prima di cambiare una riga, scrivi un test che registra cosa fa la funzione oggi. Non cosa dovrebbe fare. Questo è un test di caratterizzazione: stai congelando il comportamento attuale per accorgerti nel momento in cui una modifica “sicura” non lo è.

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, sotto la soglia di spedizione gratuita di $100, + $5 di spedizione US
    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  # niente spedito, spedizione DE sotto i $100

Esegui entrambi contro la funzione non ancora toccata, così sai che i numeri attesi vengono dal codice e non da un errore di battitura. Resetta INVENTORY prima di ogni test. Ogni passo qui sotto viene eseguito contro questa stessa coppia. Finché passano, il comportamento non si è spostato.

Estrai funzioni per prezzo e spedizione

Estrai la logica di prezzo e spedizione in funzioni il cui nome dice cosa fanno. La logica in sé non cambia in questo passo.

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

Esegui i test: stessi due numeri, stesso conteggio di inventario. Fai il commit con un messaggio che dice esattamente cosa è successo: “extract price_line_item and calculate_shipping, no behavior change”.

Sostituisci i condizionali annidati con guard clause

Il controllo dello stock in process_order sono tre livelli di nidificazione intorno all’unica riga che conta. Trasforma ogni controllo in un’uscita anticipata per il caso che non può proseguire, e il percorso di successo smette di stare indentato sotto tutto il resto.

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

Stessi test, stessi risultati. Il corpo del loop ora si legge dall’alto in basso: rifiuta, rifiuta, fai il lavoro. Sono gli stessi rami di prima, un livello di profondità invece di tre.

Sostituisci l’argomento flag con una tabella di lookup

is_vip cambia cosa fa internamente price_line_item. Ogni punto di chiamata deve sapere che il booleano esiste, e un terzo livello significa un terzo ramo copia-incollato. Sposta i numeri nei dati:

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 cambia allo stesso modo: il suo parametro viene rinominato da is_vip a tier, e l’unica chiamata che fa a price_line_item(item, is_vip) diventa price_line_item(item, tier). Nient’altro nel corpo della funzione cambia. Una chiamata come process_order(order, tier="vip") si legge da sola, e un nuovo livello è una nuova voce in DISCOUNT_RATES invece di un altro ramo if/else. Questo è l’unico passo che cambia una firma, quindi i test hanno bisogno di una modifica corrispondente: passa tier="vip" invece di is_vip=True. Le asserzioni restano come sono, perché l’output per lo stesso ordine non è cambiato.

Sposta gli effetti collaterali ai margini

price_line_item ora è pura: stessi input, stesso output, niente viene modificato. process_order modifica ancora INVENTORY e chiama print in mezzo a un loop, ed è per questo che testarla richiede un reset globale invece di una semplice chiamata di funzione. Separa il decidere dal fare. Calcola cosa dovrebbe succedere, restituiscilo, e lascia che sia chi chiama ad agire di conseguenza.

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

Due dettagli fanno di questo refactoring una corrispondenza esatta con l’originale, non solo approssimativa.

Il primo: il ramo dell’ordine vuoto passa ancora per calculate_shipping invece di restituire uno 0 fisso. L’originale addebita la spedizione anche su un ordine vuoto, quindi il refactoring deve continuare ad addebitarla anche lui.

Il secondo: plan_order controlla lo stock contro una copia locale, remaining, che decrementa man mano, non direttamente contro INVENTORY. Un ordine con lo stesso SKU su due righe viene così controllato contro uno stock che diminuisce esattamente come faceva il decremento inline dell’originale. Controllare entrambe le righe contro lo stesso numero non toccato avrebbe permesso a una seconda riga di passare un controllo che la prima aveva già esaurito.

plan_order legge INVENTORY ma non ci scrive mai e non stampa nulla, quindi puoi chiamarla mille volte in un test senza resettare niente. process_order è ora l’unico punto che tocca INVENTORY o print, e ci sta in una schermata con spazio da avanzare.

Cosa guadagna la funzione rifattorizzata rispetto all’originale

La nidificazione è passata da quattro livelli a uno. Ogni funzione fa l’unico lavoro che il suo nome descrive. INVENTORY viene scritto da esattamente un solo punto, e non resta nessun ramo copia-incollato da correggere due volte. Aggiungere un livello “all’ingrosso” è una modifica di una riga a DISCOUNT_RATES.

La storia conta quanto il codice. Ora hai cinque commit (il test, poi quattro refactoring), ognuno verde contro gli stessi test di caratterizzazione. Se tra sei mesi uno sconto risulta sbagliato, git blame sulla riga del prezzo atterra su un commit abbastanza piccolo da leggere per intero. Se al merge del branch schiacci tutti e cinque in un unico commit “refactor”, perdi questo vantaggio. Git Rebase vs Merge spiega come ogni strategia di merge modella la storia che ti resta.

Quando non rifattorizzare lo spaghetti code

Un test di caratterizzazione ha bisogno che il comportamento attuale sia osservabile e valga la pena preservarlo. Se la scadenza è oggi e non hai un test su cui appoggiarti, rifattorizzare trasforma un pasticcio funzionante in un pasticcio rotto con una formattazione migliore. Rilascia la correzione, poi pianifica la pulizia come attività a sé. Non infilarla in un commit di bug-fix dove nasconde la modifica vera.

Nemmeno il codice destinato alla cancellazione merita un refactoring. Se la funzionalità verrà sostituita nel prossimo sprint, le ore vanno nella sostituzione, non a lucidare la vecchia versione per le due settimane che le restano.

Ristrutturare codice senza nessun test è una riscrittura, e non hai modo di sapere cosa ha rotto. Se non riesci a testare il comportamento attuale, il primo passo è renderlo testabile.

Scegli una funzione che eviti di aprire, scrivi un unico test di caratterizzazione contro quello che fa adesso, e fai la prima estrazione. Fai il commit prima di toccare qualsiasi altra cosa.

Articoli correlati