Abres una función para arreglar un bug de una línea y cuarenta minutos después sigues rastreando qué rama asigna qué variable. Tres tareas que no tienen nada que ver entre sí están soldadas en un solo bloque, y ninguna tiene nombre. Tiras de un hilo y no puedes saber cuáles otros se mueven.

Eso es spaghetti code, y el nombre no tiene nada que ver con la indentación ni el estilo de nombres. Describe el flujo de control: la ejecución salta entre condiciones como un fideo que cruza todos los demás en el plato, y seguir un solo camino hasta el final significa tener cuatro niveles de if en la cabeza a la vez.

Qué hace que el código sea spaghetti code

Cuatro síntomas aparecen juntos en casi todos los casos, y cada uno se puede corregir por separado:

  • Anidamiento profundo. Tres o cuatro niveles de if/else uno dentro de otro. Cada nivel añade indentación y una condición que tienes que mantener en la cabeza para saber qué línea estás leyendo.
  • Funciones que hacen varios trabajos. Una función valida la entrada, calcula un resultado, modifica el estado y escribe un log. Cambias el cálculo y arriesgas el log; pruebas la validación y tienes que simular el estado.
  • Estado compartido oculto. Una variable global o un diccionario a nivel de módulo, modificado desde dentro de una función cuyo nombre no lo insinúa. Dos llamadores pueden pisarse entre sí y el código de ninguno muestra por qué.
  • Ramas copiadas y pegadas. La misma forma de lógica repetida cambiando un solo valor, en vez de extraerla una vez. Corriges una copia, olvidas la otra, y tienes un bug que solo aparece en la mitad de las entradas.

Ninguno de estos necesita una reescritura. Cada uno tiene un refactor con nombre que lo elimina sin cambiar lo que hace el código. Esa restricción importa más de lo que parece: si detectas un bug mientras limpias, no lo corrijas en el mismo cambio. Hazlo aparte, o no podrás saber qué cambio causó qué regresión.

Una función real con todos los síntomas del spaghetti code

Aquí está process_order, del flujo de checkout de una tienda pequeña. Léela una vez y fíjate dónde pierdes el hilo.

 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

Ahí están los cuatro síntomas. La línea que cobra al cliente está a cuatro niveles de profundidad. La función revisa el stock, calcula precios, aplica un descuento de fidelidad, modifica INVENTORY, envía (con print, en lugar de una llamada de envío real) y totaliza el pedido. INVENTORY es un diccionario a nivel de módulo que cualquier otra función también puede modificar, y nada en la firma te avisa. Las ramas VIP y no VIP tienen la misma forma con números distintos, así que un tercer nivel de descuento significa una tercera copia.

Cómo refactorizar spaghetti code sin romperlo

Un cambio pequeño y mecánico a la vez. Cada uno deja el comportamiento idéntico y tiene su propio commit. Si el paso 3 rompe algo, git bisect puede encontrar cuál de los cinco commits pequeños causó el problema. Un único commit “refactorizar todo” no le da a bisect nada que acotar.

Fija el comportamiento con un test de caracterización

Antes de cambiar una línea, escribe un test que registre lo que hace la función hoy. No lo que debería hacer. Esto es un test de caracterización: estás congelando el comportamiento actual para notar el momento en que un cambio “seguro” no lo es.

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, bajo el umbral de envío gratis de $100, + $5 de envío 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  # nada enviado, envío DE bajo $100

Ejecuta ambos contra la función todavía intacta, así sabes que los números esperados vienen del código y no de un error de tipeo. Reinicia INVENTORY antes de cada test. Cada paso de abajo se ejecuta contra este mismo par. Mientras pasen, el comportamiento sigue siendo el mismo.

Extrae funciones para precio y envío

Extrae la lógica de precio y envío en funciones cuyo nombre diga qué hacen. La lógica en sí no cambia en este paso.

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

Ejecuta los tests: los mismos dos números, el mismo conteo de inventario. Haz el commit con un mensaje que diga exactamente qué pasó: “extract price_line_item and calculate_shipping, no behavior change”.

Reemplaza los condicionales anidados con guard clauses

La verificación de stock en process_order supone tres niveles de anidamiento alrededor de la única línea que importa. Convierte cada verificación en una salida temprana para el caso que no puede continuar, y el camino de éxito deja de estar indentado bajo todo lo demás.

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

Mismos tests, mismos resultados. El cuerpo del bucle ahora se lee de arriba a abajo: rechaza, rechaza, haz el trabajo. Son las mismas ramas de antes, un nivel de profundidad en vez de tres.

Reemplaza el argumento flag con una tabla de lookup

is_vip cambia qué hace internamente price_line_item. Cada punto de llamada tiene que saber que existe el booleano, y un tercer nivel significa una tercera rama copiada y pegada. Mueve los números a datos:

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 de la misma manera: su parámetro se renombra de is_vip a tier, y la única llamada que hace a price_line_item(item, is_vip) se convierte en price_line_item(item, tier). Nada más en el cuerpo de la función cambia. Una llamada como process_order(order, tier="vip") se lee por sí sola, y un nuevo nivel es una nueva entrada en DISCOUNT_RATES en vez de otro brazo if/else. Este es el único paso que cambia una firma, así que los tests necesitan un ajuste correspondiente: pasa tier="vip" en vez de is_vip=True. Las aserciones se quedan como están, porque la salida para el mismo pedido no ha cambiado.

Lleva los efectos secundarios a los bordes

price_line_item ahora es pura: mismas entradas, misma salida, nada se modifica. process_order todavía modifica INVENTORY y llama a print en medio de un bucle, por eso probarla necesita un reinicio global en vez de una simple llamada a función. Separa decidir de hacer. Calcula qué debería pasar, devuélvelo, y deja que quien llama actúe en consecuencia.

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

Dos detalles hacen que esto sea una coincidencia exacta con el original, no una aproximación. Primero, la rama del pedido vacío sigue pasando por calculate_shipping en vez de devolver un 0 fijo: el original cobra envío incluso en un pedido vacío, así que el refactor tiene que seguir cobrándolo también. Segundo, plan_order verifica el stock contra una copia local remaining que va decrementando, no directamente contra INVENTORY, así que un pedido con el mismo SKU en dos líneas se sigue verificando contra un stock decreciente, igual que hacía el decremento en línea del original. Verificar ambas líneas contra el mismo número intacto dejaría pasar una segunda línea que la primera ya debería haber agotado.

plan_order lee INVENTORY pero nunca escribe en él ni imprime, así que puedes llamarla mil veces en un test sin reiniciar nada. process_order es ahora el único lugar que toca INVENTORY o print, y cabe en una pantalla con espacio de sobra.

Qué gana la función refactorizada sobre la original

El anidamiento pasó de cuatro niveles a uno. Cada función hace el único trabajo que su nombre describe. INVENTORY se escribe desde exactamente un lugar, y no queda ninguna rama copiada y pegada que corregir dos veces. Añadir un nivel “mayorista” es un cambio de una línea en DISCOUNT_RATES.

La historia importa tanto como el código. Ahora tienes cinco commits (el test, luego cuatro refactors), cada uno en verde contra los mismos tests de caracterización. Si un descuento sale mal dentro de seis meses, git blame en la línea del precio aterriza en un commit lo bastante pequeño como para leerlo completo. Si aplastas los cinco en un único commit “refactor” al fusionar la rama, pierdes eso. Git Rebase vs Merge explica cómo cada estrategia de fusión moldea la historia que te queda.

Cuándo no refactorizar spaghetti code

Un test de caracterización necesita que el comportamiento actual sea observable y valga la pena preservarlo. Si el plazo es hoy y no tienes un test en el que apoyarte, refactorizar convierte un desastre que funciona en un desastre roto con mejor formato. Publica la corrección, luego planifica la limpieza como una tarea aparte. No la metas de contrabando en un commit de bug-fix donde oculte el cambio real.

Tampoco el código programado para eliminarse merece un refactor. Si la funcionalidad va a reemplazarse el próximo sprint, las horas van al reemplazo, no a pulir la versión vieja durante las dos semanas que le quedan.

Reestructurar código sin ningún test es una reescritura, y no tienes forma de saber qué rompiste. Si no puedes probar el comportamiento actual, el primer paso es hacerlo testeable.

Elige una función que evites abrir, escribe un único test de caracterización contra lo que hace ahora mismo, y haz la primera extracción. Haz el commit antes de tocar cualquier otra cosa.

Artículos relacionados