Módulo 7: Patrones como vocabulario de revisión
3. God object, feature envy, shotgun surgery
Descripción
Al terminar esta lección vas a poder detectar los tres olores que más aparecen en revisiones reales y que más caro cuestan, y vas a poder hacerlo con evidencia contable en vez de con impresiones. Para cada uno vas a salir con cuatro cosas: qué es exactamente, cómo se ve por dentro, un método de detección que se puede ejecutar en dos minutos, y hacia dónde suele resolverse. Los tres tienen su ejemplar vivo en Boletia, y los tres van a reaparecer en el proyecto del módulo.
Esto importa porque estos tres concentran la mayor parte del dolor de mantenimiento en sistemas medianos: son los que hacen que un equipo pase de "avanzamos rápido" a "cada cambio nos toma el triple". God object es la razón por la que dos personas no pueden trabajar en paralelo sin chocar. Feature envy es la razón por la que la lógica del negocio termina desperdigada donde nadie la busca. Y shotgun surgery es la razón por la que un cambio de una línea conceptual se convierte en un PR de cinco archivos con un modo de falla silencioso.
Hay una razón adicional para dedicarles una lección entera: son los tres que se pueden verificar. Sus métodos de detección producen números, y un número es lo único que termina rápido una discusión. Cuando escribas "este archivo apareció en trece de los últimos doscientos commits", nadie va a responder "es tu opinión".
Conexión con el módulo: la lección 2 instaló el concepto de olor y te dio el catálogo con doce entradas de una línea cada una. Esta lección toma tres de esas entradas y las convierte en herramientas de trabajo. El orden no es casual: God object es un olor de tamaño, feature envy es uno de ubicación y shotgun surgery es uno de cambio — un ejemplar de cada familia, y los tres están relacionados entre sí de una forma que vamos a ver al final. La lección 4 toma lo que aquí detectas y lo convierte en un comentario sobre el que el autor pueda actuar. Y la lección 6 recorre el camino de salida: cómo se refactoriza hacia el patrón que cada uno pide, en pasos que no rompen nada.
God object: la persona que sabe dónde está todo
En toda oficina que creció rápido hay alguien así. Empezó de asistente hace ocho años, cuando eran cinco personas, y como era la única disponible fue absorbiendo cosas: las llaves, los proveedores, la contraseña del portal del banco, el trato con el contador, quién arregla la impresora, dónde está el contrato de arrendamiento. Hoy son cuarenta personas y esa persona es indispensable. Todo el mundo pasa por su escritorio.
Fíjate en las cuatro consecuencias, porque son exactamente las de un God object:
Es un cuello de botella. Si dos personas la necesitan a la vez, una espera. En código: dos desarrolladores tocando el mismo archivo la misma semana es un conflicto de merge.
Nadie más sabe nada. Para entender cómo funciona el sistema hay que leer ese archivo, porque es el único que sabe cómo se conectan las piezas.
Si se va de vacaciones, la oficina se detiene. En código: cualquier cambio ahí tiene riesgo desproporcionado, así que la gente le tiene miedo, así que nadie lo mejora, así que sigue creciendo.
Crece sola. Cuando aparece una tarea nueva y nadie sabe de quién es, se la dan a ella. En código esto es lo más importante: un God object atrae código. Como ya tiene acceso a todo lo que hace falta, meter una cosa más siempre parece la opción barata.
Nadie diseñó esa situación. Cada decisión individual fue razonable. Los God objects no se escriben: se acumulan.
Qué es exactamente
Un God object es una clase, módulo o función que concentra tantas responsabilidades que se vuelve el punto por el que pasa todo el sistema. Las señales que lo acompañan son: muchos colaboradores (importa mucho), mucho estado, muchos métodos sin relación entre sí, y —la más diagnóstica— una tasa de cambio muy alta.
Fowler lo llama Large Class; la comunidad, God object o Blob. Uso "God object" porque es el que más vas a oír en una revisión.
Un matiz de entrada, porque es la fuente del falso positivo más común: un archivo grande no es automáticamente un God object. Un módulo de mil líneas que solo contiene constantes, o un archivo de rutas con cuarenta endpoints, es grande y está bien. Lo que define al God object no es el tamaño: es la variedad de razones por las que cambia. Por eso está tan emparentado con divergent change.
En Boletia: checkout/checkout.py
Aquí está, resumido. En el original son unas trescientas líneas.
# Archivo: checkout/checkout.py
def checkout(order, coupon=None):
tickets = [repository.get_ticket(tid) for tid in order.ticket_ids]
# ---- 1. Precio -------------------------------------------------
subtotal = 0.0
for ticket in tickets:
subtotal += calculate_price(ticket, order.created_at)
if coupon:
subtotal = subtotal * (1 - COUPON_RATES[coupon])
fee = subtotal * SERVICE_FEE_RATE
order.total = round(subtotal + fee, 2)
# ---- 2. Asientos ------------------------------------------------
event = repository.get_event(tickets[0].event_id)
if event.has_numbered_seats:
plugin = SeatingPluginRegistry.get(settings.SEATING_PLUGIN)
for ticket in tickets:
ticket.seat = plugin.assign(event, order).code
# ---- 3. Cobro ---------------------------------------------------
if order.provider == "stripe":
provider = StripeProvider(StripeClient(api_key=settings.STRIPE_KEY))
elif order.provider == "mercadopago":
provider = MercadoPagoProvider(MercadoPagoClient(token=settings.MP_TOKEN))
elif order.provider == "cash":
provider = CashProvider()
else:
raise ValueError(f"Proveedor desconocido: {order.provider}")
result = provider.charge(order)
if not result.ok:
order.status = "cancelled"
repository.save_order(order)
raise PaymentFailed(order.id, result.reference)
order.status = "paid"
repository.save_order(order)
# ---- 4. Avisos ---------------------------------------------------
customer = repository.get_customer(order.customer_id)
email_channel.send(customer.email, build_confirmation(order))
if customer.phone:
sms_channel.send(customer.phone, build_short_confirmation(order))
if customer.push_token:
push_channel.send(customer.push_token, build_push_confirmation(order))
organizer = repository.get_customer(event.organizer_id)
email_channel.send(organizer.email, build_organizer_alert(order, event))
analytics.track("order_paid", order_id=order.id, total=order.total)
return order
Fíjate en los comentarios numerados. El autor los puso, y eso es en sí mismo una señal: cuando alguien tiene que dividir una función con comentarios de sección, está admitiendo que la función hace cuatro cosas. Un comentario que dice # ---- 3. Cobro ---- es un nombre de función esperando a nacer.
Cada uno de esos bloques cambia por una razón distinta y a cargo de gente distinta:
| Bloque | Cambia cuando | Quién lo pide |
|---|---|---|
| 1. Precio | Cambia una tarifa, entra un tipo de boleto, se agrega un tipo de cupón | Producto / finanzas |
| 2. Asientos | Cambia la asignación de lugares, entra un venue con reglas raras | Operaciones |
| 3. Cobro | Entra un proveedor, cambia un SDK, cambia el manejo de fallos | Pagos |
| 4. Avisos | Entra un canal, cambia un texto, entra un nuevo interesado | Marketing / soporte |
Cuatro razones de cambio en un archivo. Y las cuatro áreas pueden moverse en el mismo sprint.
Cómo se detecta: cuatro cosas que se cuentan
Este es el método. No requiere herramientas especiales y toma dos minutos.
Uno: cuenta los imports. Es la medida más barata de "cuántas cosas necesita saber este archivo".
# Cuántos módulos del propio proyecto importa el archivo sospechoso.
$ grep -c "^from\|^import" checkout/checkout.py
14
Catorce dependencias en un archivo. No hay un número mágico, pero cuando pasa de diez vale la pena mirar por qué.
Dos: cuenta las razones de cambio. Lee el archivo y pregúntate: "¿cuántas cosas distintas del negocio tendrían que pasar para que este archivo se modifique?". Si la respuesta es una, está bien. Si son cuatro, es divergent change y muy probablemente God object.
Tres: mira el historial. Esta es la evidencia más fuerte que existe y casi nadie la usa en revisiones.
# Los archivos que más veces se tocaron en los últimos 200 commits.
$ git log --format=format: --name-only -n 200 | sort | uniq -c | sort -rg | head -8
13 checkout/checkout.py
9 api/routes.py
7 pricing/calculator.py
6 config.py
5 notifications/notifier.py
4 payments/stripe_provider.py
3 models/order.py
3 reports/csv_exporter.py
Trece de los últimos doscientos commits tocaron checkout.py. Ese número no es una opinión sobre el diseño: es un hecho sobre cómo se comporta el equipo alrededor de ese archivo. Y tiene una lectura directa que puedes poner en un comentario: "uno de cada quince cambios del sistema pasa por aquí, así que uno de cada quince cambios tiene el riesgo del archivo más delicado".
Cuatro: pregunta por los conflictos. "¿Cuándo fue la última vez que dos personas chocaron en este archivo?". Si la respuesta es "siempre", tienes tu caso sin argumentar nada sobre diseño.
Hacia dónde se resuelve
La dirección general es extraer colaboradores: sacar cada responsabilidad a un lugar donde viva sola, y dejar en el orquestador solo el orden de los pasos. En Boletia, cada bloque apunta a un patrón que ya viste:
- Bloque 1 (precio) → Strategy por tipo de boleto (módulo 3). El
checkoutpidepricing.total_for(order)y no se entera de las reglas. - Bloque 3 (cobro) → Factory que devuelve el
PaymentProvidercorrecto (módulo 4). Elcheckoutpidepayments.provider_for(order)y no se entera de los SDKs. - Bloque 4 (avisos) → Observer o simplemente una llamada a
notify(order)(módulo 6). Elcheckoutavisa que la orden se pagó y no se entera de los canales.
Después de esos tres movimientos, checkout() queda en unas quince líneas que se leen como una lista de pasos, y el archivo pasa a tener una sola razón de cambio: que cambie el orden o la composición del flujo de compra.
Ahora la advertencia, tan importante como la dirección. Partir un God object en cinco God objects chiquitos no es progreso. El error clásico es cortar por tamaño —"tiene 300 líneas, hagamos seis de 50"— en vez de cortar por razón de cambio. La prueba: cada pieza nueva debería describirse en una frase que no contenga la palabra "y". Si dices "esta clase calcula el precio y notifica", no cortaste, moviste. Y lo segundo, que viene directo del módulo 2: el God object no se desarma de un PR. Se desarma extrayendo una responsabilidad a la vez, con el sistema funcionando después de cada paso; la lección 6 muestra esa secuencia.
Feature envy: el vecino que vive en tu cocina
Tienes un vecino que técnicamente vive en el departamento de al lado. Pero desayuna en tu cocina, guarda su comida en tu refrigerador, usa tu horno, come en tu mesa y lava sus platos en tu fregadero. Solo va a su casa a dormir.
La pregunta obvia: ¿por qué no vive aquí? No hay una respuesta buena. La situación es puro costo: cada vez que pasa algo con tu cocina —la cambias de lugar, compras un refri distinto, remodelas— él se entera y se ve afectado, aunque no sea su cocina. Y si alguien quiere saber "quién usa esta cocina", la respuesta no es obvia.
Eso es feature envy. Un método que vive en una clase pero usa casi exclusivamente datos de otra. La solución suele ser tan simple como suena: mudarlo.
Qué es exactamente
Feature envy es un método que accede a más datos de otro objeto que de aquel al que pertenece. El nombre es literal: el método "envidia" los datos de otra clase, y esa envidia es una señal de que está en el lugar equivocado.
La anatomía tiene tres piezas:
- Un método en una clase
A. - Que usa
natributos o métodos de una claseB. - Y usa muy pocos —a veces cero— de
A.
Cuando n es grande y lo propio es cero, el diagnóstico es casi seguro. Cuando están parejos, hay que pensar.
Por qué importa, más allá de la estética: la lógica que decide sobre los datos de Order debería vivir cerca de Order. Si está repartida en cinco clases distintas, entonces para entender qué se puede hacer con una orden hay que buscar en cinco archivos, y para cambiar una regla hay que encontrarlos todos. Es el mismo problema de dispersión que veremos en shotgun surgery, en su versión local.
En Boletia: notify_finance
Este método existe en el módulo de conciliación que alguien agregó hace unos meses:
# Archivo: reports/reconciliation.py
class ReconciliationReport:
def __init__(self, period, output_dir):
self.period = period
self.output_dir = output_dir
def notify_finance(self, order):
# Arma el aviso que se le manda a finanzas cuando una orden
# necesita revisión manual (efectivo vencido, monto raro, etc.).
subject = f"Revisión manual — orden #{order.id}"
body = (
f"Cliente: {order.customer_id}\n"
f"Total: {order.total}\n"
f"Proveedor: {order.provider}\n"
f"Estado: {order.status}\n"
f"Creada: {order.created_at}\n"
f"Boletos: {len(order.ticket_ids)}\n"
f"Referencia externa: {order.external_ref}\n"
)
if order.provider == "cash" and order.status == "pending":
body += "Pago en efectivo vencido.\n"
mailer.send(FINANCE_EMAIL, subject, body)
Cuenta con el dedo. Accesos a order: id, total, provider, status, created_at, ticket_ids, external_ref, más provider y status otra vez en el if. Nueve accesos a Order. Accesos a self: cero. El método está en ReconciliationReport y no usa ni self.period ni self.output_dir.
Esta clase no necesita este método. Este método necesita a Order.
Cómo se detecta: cuenta y compara
El método de detección es literalmente contar, y se puede hacer a ojo mientras lees.
Regla de bolsillo: en el cuerpo del método, cuenta los accesos que empiezan con self. y los que empiezan con el nombre de un parámetro. Si los del parámetro superan claramente a los propios, hay envidia.
# Un contador mental, aplicado al método de arriba:
# self.* → 0
# order.* → 9
# Veredicto: feature envy hacia Order. Y no es un caso limítrofe.
Dos variantes que conviene reconocer porque se detectan igual. Envidia repartida: el método usa datos de dos clases ajenas y de ninguna propia —cinco atributos de Order y cuatro de Customer—. Aquí mudarlo no es obvio, y la respuesta suele ser que el método revela un concepto que falta. Envidia de comportamiento: el método llama muchos métodos de otro objeto en vez de leer sus atributos; order.mark_paid(); order.recalculate(); order.save() está manejando el ciclo de vida de una orden desde afuera.
Y dos falsos positivos que hay que saber descartar. El primero: cuando el objeto envidiado es una estructura de datos. Si Order fuera un DTO —un registro que viaja entre capas—, es normal que otras clases lean sus campos. La pregunta que separa los casos: ¿este objeto tiene reglas propias o solo transporta valores? En Boletia, Order tiene estados y transiciones: no es un DTO. El segundo, más sutil: la envidia hacia un objeto que no controlas. Usar ocho atributos de la respuesta de un SDK externo no es feature envy —no puedes mover código dentro de la librería de Stripe—; ese caso pide un Adapter (módulo 5).
Hacia dónde se resuelve
La refactorización se llama, sin sorpresa, mover el método (Move Method): el método se va a la clase cuyos datos usa. En el caso de arriba:
# Archivo: models/order.py
class Order:
...
def finance_review_summary(self):
# Ahora vive donde viven los datos que necesita.
# Nota que ya no hace falta pasar nada: usa lo propio.
lines = [
f"Cliente: {self.customer_id}",
f"Total: {self.total}",
f"Proveedor: {self.provider}",
f"Estado: {self.status}",
f"Creada: {self.created_at}",
f"Boletos: {len(self.ticket_ids)}",
f"Referencia externa: {self.external_ref}",
]
if self.is_expired_cash_payment():
lines.append("Pago en efectivo vencido.")
return "\n".join(lines)
def is_expired_cash_payment(self):
# Una regla del dominio que estaba escondida en un `if` de reportes.
return self.provider == "cash" and self.status == "pending"
# Archivo: reports/reconciliation.py
class ReconciliationReport:
def notify_finance(self, order):
# Lo que queda aquí es lo que de verdad le toca a un reporte: mandar el correo.
mailer.send(FINANCE_EMAIL, f"Revisión manual — orden #{order.id}",
order.finance_review_summary())
Mira lo que pasó, porque es más que un movimiento de líneas. Al mudar el método apareció una regla del negocio que no tenía nombre: is_expired_cash_payment(). Estaba escondida dentro de un if en un archivo de reportes, donde nadie que trabajara en pagos la habría buscado nunca. Ese es el beneficio real de corregir feature envy: no es ordenar, es descubrir conceptos del dominio que estaban disueltos en el código de otro lado.
Y una advertencia de dosis: mover método por método hasta que todo el comportamiento viva en los modelos puede llevarte al extremo contrario —modelos gigantes que hacen de todo, es decir, God objects—. La regla de equilibrio: el comportamiento que depende solo de los datos de una clase pertenece a esa clase; el comportamiento que coordina varias clases pertenece a un coordinador. finance_review_summary() solo necesita datos de la orden, así que se muda. Mandar el correo necesita el mailer y la dirección de finanzas, así que se queda afuera.
Shotgun surgery: el número de teléfono anotado en doce lugares
Cambias de número de teléfono. Debería ser un trámite de un minuto.
Pero tu número está en el contrato del gimnasio, en la ficha del veterinario, en la cuenta del banco, en tres formularios de la escuela de tu hija, en la agenda de contactos de tu suegra, en la etiqueta de la maleta y en el aviso de "se busca gato" que sigue pegado en el poste de la esquina. Un cambio conceptual —"mi número es otro"— se convierte en doce trámites. Y el problema no es el trabajo: es que te vas a olvidar de uno, y el que se te olvide va a fallar en el peor momento posible, cuando el veterinario intente avisarte algo urgente.
Eso es shotgun surgery. El nombre viene de la imagen de una escopeta: un solo disparo, perdigones repartidos por todas partes.
Qué es exactamente
Shotgun surgery es cuando un solo cambio conceptual obliga a modificar muchos archivos distintos. El síntoma se mide en el trabajo, no en el código: no se ve leyendo un archivo, se ve intentando hacer un cambio.
Es el opuesto simétrico de divergent change, y conviene tener la simetría clara:
| Un tipo de cambio | Muchos tipos de cambio | |
|---|---|---|
| Toca un archivo | Lo normal y deseable | Divergent change — el archivo hace demasiado |
| Toca muchos archivos | Shotgun surgery — el concepto está disperso | Lo normal en un sistema grande |
Los dos son problemas de frontera mal puesta. En divergent change, una frontera agrupa cosas que no van juntas. En shotgun surgery, una frontera separa cosas que sí van juntas. Y por eso los dos suelen convivir en el mismo sistema: checkout.py sufre divergent change (cambia por cuatro razones) y el concepto "proveedor de pago" sufre shotgun surgery (vive en cinco archivos).
Lo que hace a este olor el más caro de los tres es su modo de falla. Un God object es lento de trabajar pero visible. Feature envy es desordenado pero inofensivo. Shotgun surgery produce bugs silenciosos: el cambio incompleto compila, pasa las pruebas existentes y se comporta mal solo en el caso que nadie probó.
Ejemplo trabajado: la prueba de los cinco archivos, en vivo
Vamos a hacer el conteo, que es la técnica central de esta sección. El cambio conceptual es uno: "Boletia ahora también cobra con Klarpay".
Cambio conceptual: "soportamos un proveedor de pago más"
│
├─ 1. payments/klarpay_provider.py (nuevo) la implementación
├─ 2. checkout/checkout.py:41 (+3) un elif más en el corazón del sistema
├─ 3. config.py:18 (+1) la credencial en el Settings
├─ 4. api/routes.py:66 (+1) "klarpay" en ALLOWED_PROVIDERS
└─ 5. tests/test_checkout.py (+n) el caso nuevo
Cinco lugares. Y hay un sexto, medio escondido, que aparece solo si alguien se acuerda:
6. reports/reconciliation.py:73 PROVIDER_LABELS = {"stripe": ..., "mercadopago": ..., "cash": ...}
← si no lo tocas, el reporte muestra "klarpay" en crudo
Ahora lo importante: qué pasa si se te olvida uno.
| Si olvidas | Qué pasa | Cuándo te enteras |
|---|---|---|
El elif de checkout.py | ValueError: Proveedor desconocido en el corazón del checkout | Al primer intento de compra. Ruidoso, pero en producción |
La lista de routes.py | La API rechaza el proveedor con un 400, aunque el checkout sí sabría cobrarlo | Al primer intento. Confuso: el error apunta al lugar equivocado |
La credencial en config.py | KeyError: 'KLARPAY_KEY' al arrancar el proceso, o peor, al primer cobro | Al desplegar, si hay suerte |
La etiqueta de reconciliation.py | El reporte de finanzas muestra klarpay sin traducir. Nada falla | Nunca, hasta que alguien de finanzas se queje |
Qué esperar de este conteo. Lo primero: la tabla es el comentario. Cuando lleves esto a una revisión no digas "esto es shotgun surgery"; pega la última columna. "Nunca, hasta que alguien de finanzas se queje" es el argumento entero, y no admite discusión.
Lo segundo, y es la fila que hace caro al olor: un cambio incompleto que no rompe nada es el peor resultado posible. Los tres primeros modos de falla son ruidosos y molestos; el cuarto deja el sistema en un estado inconsistente que nadie va a detectar hasta que el daño ya ocurrió. Al escribir la consecuencia, esa asimetría es lo que hay que subrayar — porque la reacción natural del autor va a ser "pero funciona", y sí, funciona: ese es el problema.
Lo tercero: el sexto proveedor va a repetir exactamente el mismo recorrido. Eso es lo que convierte un hallazgo anecdótico en uno estructural. No es que este cambio haya sido descuidado; es que el concepto "proveedor de pago" no tiene un lugar propio en el sistema, y por eso cada aparición nueva se reparte igual.
Cómo se detecta: la prueba del cambio y el historial
Hay dos técnicas, y las dos producen números.
Técnica 1: la prueba del cambio hipotético. Es la que más vas a usar en revisiones. Elige un cambio plausible que el equipo vaya a necesitar y recorre mentalmente —o con grep— qué archivos habría que tocar.
# ¿Dónde vive el concepto "proveedor de pago"?
$ grep -rln "stripe\|mercadopago\|provider" --include="*.py" . | sort
api/routes.py
checkout/checkout.py
config.py
models/order.py
payments/cash_provider.py
payments/mercadopago_provider.py
payments/provider.py
payments/stripe_provider.py
reports/reconciliation.py
tests/test_checkout.py
Diez archivos mencionan el concepto. Los cuatro de payments/ son legítimos —ahí es donde debe vivir—. Los otros seis son la dispersión.
La forma útil de decirlo en un comentario no es "diez archivos mencionan el proveedor". Es: "para agregar el próximo proveedor hay que tocar cinco archivos fuera de payments/, y olvidar uno de ellos no rompe las pruebas". Un escenario concreto con un modo de falla concreto.
Técnica 2: los archivos que cambian juntos. Esta es la evidencia histórica y es muy convincente porque no admite discusión. La idea: si dos archivos aparecen juntos en la mayoría de los commits que los tocan, es que están acoplados aunque nada en el código lo diga.
# Los commits que tocaron checkout.py: ¿qué otros archivos venían en el mismo commit?
$ git log --format="%h" -- checkout/checkout.py | \
while read c; do git show --format= --name-only "$c"; done | \
sort | uniq -c | sort -rg | head -6
13 checkout/checkout.py
8 config.py
7 api/routes.py
6 tests/test_checkout.py
5 payments/provider.py
2 pricing/calculator.py
Ocho de los trece cambios a checkout.py vinieron acompañados de un cambio a config.py. Siete de trece, de uno a routes.py. Ese acoplamiento no aparece en ningún import: es acoplamiento lógico, y solo se ve en el historial. Es exactamente la evidencia que convierte una discusión de opiniones en una de datos.
Hacia dónde se resuelve
La dirección general es darle al concepto disperso un solo lugar donde vivir, y hacer que los demás archivos lo consulten en vez de repetirlo. En Boletia, el movimiento concreto es un registro de proveedores en payments/:
# Archivo: payments/registry.py
# Un solo lugar que sabe qué proveedores existen y cómo se construyen.
# Lo que antes estaba en cinco archivos, ahora está aquí.
_PROVIDERS = {
"stripe": lambda: StripeProvider(StripeClient(api_key=settings.STRIPE_KEY)),
"mercadopago": lambda: MercadoPagoProvider(MercadoPagoClient(token=settings.MP_TOKEN)),
"cash": lambda: CashProvider(),
}
# Etiquetas para reportes: viven junto a la definición, no en reports/.
LABELS = {"stripe": "Tarjeta (Stripe)", "mercadopago": "MercadoPago", "cash": "Efectivo"}
def provider_for(name):
if name not in _PROVIDERS:
raise UnknownProvider(name)
return _PROVIDERS[name]()
def available():
# routes.py ya no mantiene su propia lista: pregunta aquí.
return tuple(_PROVIDERS)
Con eso, los cinco lugares se reducen a dos: el archivo del proveedor nuevo y una línea en _PROVIDERS. checkout.py pierde su if/elif completo y queda con provider = payments.provider_for(order.provider). routes.py pierde su lista blanca y llama a payments.available(). reconciliation.py pierde su diccionario de etiquetas y usa payments.LABELS.
Eso es lo que en el módulo 4 llamamos Factory, y el mecanismo del diccionario es su forma más simple —no hace falta una jerarquía de clases para tenerlo—.
Ahora la advertencia, que viene del módulo 2. No todo shotgun surgery se arregla, y no todo se arregla con un patrón. Si el cambio ocurre una vez al año, cinco archivos son un costo aceptable comparado con el costo permanente de una indirección más; la regla de tres aplica aquí igual que en todas partes. Y, más importante: a veces la dispersión es real y no accidental. Que agregar un proveedor obligue a escribir una prueba es correcto. El objetivo no es que un cambio toque un solo archivo —eso es imposible—, es que no toque archivos que no tendrían por qué enterarse.
Los tres juntos: cómo se relacionan
Vale la pena verlos como un sistema, porque en código real aparecen encadenados y entender la cadena te ahorra diagnosticar el eslabón equivocado.
God object y divergent change son el mismo fenómeno visto desde dos ángulos: demasiadas responsabilidades en un lugar. Uno se ve leyendo el archivo; el otro, en el historial.
Shotgun surgery es el mismo problema en el otro sentido: un concepto que no tiene lugar propio se desparrama. Y aquí está la conexión que importa: un God object produce shotgun surgery en los conceptos que absorbió. Como checkout.py se quedó con la decisión de qué proveedor usar, el concepto "proveedor" quedó partido entre payments/ (las implementaciones) y checkout.py (la decisión). Arreglar uno arregla parte del otro.
Feature envy es la versión local del mismo problema: lógica que vive lejos de sus datos. Por eso es el más barato de corregir: mover un método es mecánico y de bajo riesgo.
De ahí sale un orden de ataque que sirve como criterio de priorización en una revisión: feature envy primero (barato, seguro, y suele revelar conceptos escondidos como is_expired_cash_payment); shotgun surgery después (más caro, pero es el del modo de falla silencioso, así que es el que más riesgo quita); y God object al final y por partes, porque es el más riesgoso y buena parte se resuelve como consecuencia de los dos anteriores.
Errores comunes
Diagnosticar God object por el tamaño del archivo (de método). Qué pasa: alguien ve un archivo de seiscientas líneas y escribe "esto es un God object, hay que partirlo". Resulta que era un módulo de constantes, o un archivo de rutas, o una tabla de configuración —cosas grandes que están perfectamente bien—. El comentario se descarta, y con él pierde credibilidad el siguiente comentario del mismo revisor, que sí era bueno. Por qué pasa: el tamaño es lo único que se ve sin leer, y este olor tiene fama de ser "el del archivo grande". Cómo detectarlo: si tu evidencia es un número de líneas y nada más, no investigaste. Cómo corregirlo: la señal que define este olor no es el tamaño sino la variedad de razones de cambio. Antes de escribir el comentario, contesta: ¿cuántas cosas distintas del negocio harían que este archivo se modifique? Y respalda con el historial: git log sobre ese archivo te dice en dos minutos si el equipo lo está tocando por motivos que no tienen relación.
Corregir feature envy moviendo el método a la clase equivocada (de criterio). Qué pasa: alguien detecta correctamente que un método usa datos de otra clase y lo mueve ahí, sin notar que el método usaba datos de dos clases ajenas. El resultado es que ahora la clase destino importa a la tercera, y el acoplamiento empeoró: antes había un método incómodo, ahora hay dos modelos amarrados. Por qué pasa: la refactorización "mover método" es mecánica y la hace el editor, así que se aplica sin pensar en cuál es el destino correcto. Cómo detectarlo: después de mover, cuenta los imports de la clase destino. Si creció y ahora depende de algo de lo que no dependía, mira otra vez. Cómo corregirlo: cuando la envidia está repartida entre dos clases, la respuesta casi nunca es mudar el método; es que falta un concepto. Un método que necesita datos de Order y de Customer para armar un resumen probablemente está pidiendo que exista un OrderSummary. Crear ese concepto es más trabajo que mover el método, y es la respuesta correcta.
Proponer arreglar shotgun surgery dentro del PR que lo reveló (de proceso). Qué pasa: alguien revisa un PR que agrega el cuarto proveedor de pago, detecta correctamente el olor, y pide que el autor introduzca un registro de proveedores antes de aprobar. El autor —que venía a entregar una funcionalidad— se encuentra con que su PR de cincuenta líneas se convirtió en un rediseño de doscientas que toca el corazón del sistema, con el riesgo que eso implica y sin haberlo planeado. El PR se atasca dos semanas. Por qué pasa: el olor se hace visible justo cuando alguien hace el cambio que lo revela, así que el momento se siente como el correcto. Y además hay algo de verdad en la intuición: si no se hace ahora, se va a repetir. Cómo detectarlo: si tu comentario le pide al autor un cambio que es más grande que el PR original, eso solo ya es la señal. Cómo corregirlo: separa el hallazgo del bloqueo. El olor estructural se documenta con todo detalle y se marca como no bloqueante, con un ticket. Lo que sí puede ser bloqueante es el pedazo pequeño y urgente —por ejemplo, que este PR sí actualice las cinco listas y no cuatro—. Esa separación es exactamente la que hiciste en el ejemplo de la lección 2, y la lección 4 la va a convertir en una parte fija del formato.
Ejercicios
Ejercicio 1 — Cuenta la envidia. Aquí hay un método del módulo de reportes de Boletia. Determina si hay feature envy, con el conteo explícito. Si lo hay, di a qué clase debería mudarse el método y qué concepto del dominio aparece al mudarlo.
# Archivo: reports/attendees.py
class AttendeeReport:
def __init__(self, event_id, output_dir):
self.event_id = event_id
self.output_dir = output_dir
def can_check_in(self, ticket, order):
if order.status != "paid":
return False
if ticket.status == "sold" and ticket.event_id == self.event_id:
if ticket.kind == "courtesy" and order.total > 0:
return False
return True
return False
Ver solución
El conteo. Accesos a order: status, total → 2. Accesos a ticket: status, event_id, kind → 3. Accesos a self: event_id → 1.
Cinco accesos ajenos contra uno propio. Sí hay feature envy, aunque es un caso más interesante que el del texto porque la envidia está repartida entre dos objetos y sí usa algo propio.
Adónde mudarlo. No es obvio, y ahí está la enseñanza. Mover el método entero a Ticket lo obligaría a conocer Order; moverlo a Order lo obligaría a conocer Ticket. Cualquiera de las dos opciones acopla dos modelos que hoy no se conocen.
La lectura correcta es que el método está preguntando tres cosas distintas que sí tienen dueño:
# models/order.py
def is_paid(self):
return self.status == "paid"
# models/ticket.py
def belongs_to_event(self, event_id):
return self.event_id == event_id
def is_sold(self):
return self.status == "sold"
Y queda una cuarta pregunta que no es de ninguno de los dos: ticket.kind == "courtesy" and order.total > 0. Esa condición dice algo del negocio que no tiene nombre: una cortesía que se cobró es inconsistente y no debe permitir acceso. Es una regla que cruza los dos objetos, así que su lugar natural es donde se cruzan —en el propio can_check_in, ya reducido a coordinar—:
def can_check_in(self, ticket, order):
if not order.is_paid():
return False
if not (ticket.is_sold() and ticket.belongs_to_event(self.event_id)):
return False
# Una cortesía con total cobrado es una inconsistencia: no se admite.
return not (ticket.is_courtesy() and order.total > 0)
El concepto que apareció: "cortesía cobrada", una inconsistencia de datos que estaba enterrada en un if anidado dentro de un archivo de reportes. Nadie de pagos la habría encontrado ahí.
Por qué funciona: el ejercicio te obliga a resistir la respuesta automática ("hay envidia → mueve el método"). Cuando la envidia está repartida, mover es la respuesta equivocada; la respuesta es descomponer la envidia en preguntas que sí tienen dueño, y ver qué queda al final. Lo que queda suele ser el concepto valioso.
Ejercicio 2 — Haz la prueba del cambio. Boletia quiere agregar un cuarto canal de notificación: WhatsApp. Con lo que sabes de la estructura del sistema —notifications/ tiene channel.py, email_channel.py, sms_channel.py, push_channel.py y notifier.py; checkout.py llama a los canales directamente; Customer tiene phone y push_token— enumera los archivos que habría que tocar y, para cada uno, di qué pasa si se olvida. Después decide si esto es shotgun surgery y qué le dirías al equipo.
Ver solución
El recorrido.
notifications/whatsapp_channel.py(nuevo) — la implementación. Si falta, no hay nada que hacer; es el trabajo mismo.models/customer.py— un campowhatsapp_number. Si falta, no hay a dónde mandar. Falla ruidosamente en desarrollo.notifications/notifier.py— agregar el canal a la listaCHANNELS. Si falta, el canal existe pero el recordatorio de eventos nunca lo usa. Falla en silencio.checkout/checkout.py— agregar el bloqueif customer.whatsapp_number: whatsapp_channel.send(...). Si falta, las confirmaciones de compra no salen por WhatsApp aunque el canal esté listo. Falla en silencio, y es el caso principal.- La migración de base de datos para el campo nuevo. Si falta, explota al arrancar.
api/routes.py— el endpoint de perfil, para que el cliente pueda guardar su número. Si falta, el campo existe y siempre está vacío. Falla en silencio.
¿Es shotgun surgery? Sí, y con un agravante: tres de los seis modos de falla son silenciosos, y uno de ellos —el punto 4— deja el sistema en el estado más confuso posible: el canal está implementado, probado y configurado, y aun así el usuario no recibe nada. Alguien va a perder medio día investigando.
El agravante estructural. Fíjate en los puntos 3 y 4 juntos. notifier.py existe justamente para saber por qué canales avisarle a un cliente, pero checkout.py no lo usa: llama a los canales directo. Eso significa que hay dos lugares que responden la misma pregunta —"¿por dónde le aviso a este cliente?"— y hay que acordarse de los dos. Ese es el hallazgo que vale la pena poner en el comentario, más que la lista de seis archivos.
Qué le dirías al equipo. Algo así:
Agregar un canal de notificación toca hoy seis lugares, y tres de ellos fallan en silencio si se olvidan. El más grave es que
checkout.pyno usanotifier.py: llama a los canales uno por uno, así que la pregunta "¿por dónde le aviso a este cliente?" está contestada en dos lugares distintos y hay que acordarse de los dos. Antes de meter WhatsApp vale la pena que el checkout pase pornotify(customer, message)y que la lista de canales viva solo ennotifier.py. Con eso, agregar el quinto canal sería un archivo nuevo más una línea.
Por qué funciona: la prueba del cambio es la única forma de detectar shotgun surgery, porque el olor no se ve leyendo un archivo — se ve intentando hacer algo. Y fíjate en que el recorrido produjo un hallazgo que no estaba en la lista de archivos: la duplicación de la decisión entre checkout y notifier. Eso pasa a menudo, y es la razón por la que vale la pena hacer el recorrido completo aunque parezca obvio.
Ejercicio 3 — Ordena el ataque. Un compañero te manda esta lista de hallazgos sobre el mismo módulo de Boletia y te pregunta por dónde empezar. Ordénalos y justifica el orden en función de costo, riesgo y de cuánto ayuda cada uno a los demás.
- (a)
checkout.pyes un God object: cuatro responsabilidades, trece de los últimos doscientos commits. - (b)
ReconciliationReport.notify_financees feature envy haciaOrder: nueve accesos ajenos, cero propios. - (c) Agregar un proveedor de pago es shotgun surgery: cinco archivos, uno de ellos falla en silencio.
- (d)
Order.provideres unstrlibre con dos listas blancas en archivos distintos.
Ver solución
Un orden defendible: (b) → (d) → (c) → (a).
(b) primero. Es el más barato y el de menor riesgo: mover un método es mecánico y reversible, no cambia comportamiento y se revisa en tres minutos. Además tiene un rendimiento inmediato que no se ve en el diagnóstico: al mudarlo apareció is_expired_cash_payment(), una regla de negocio que estaba escondida. Empezar por lo barato también tiene un efecto de equipo: un refactor pequeño que sale bien compra confianza para proponer el siguiente.
(d) segundo. Es pequeño en código y grande en consecuencias. Unificar las dos listas blancas en un solo lugar —el registro de payments/— es un cambio acotado, y es un prerrequisito de (c): no tiene sentido montar un registro de proveedores si va a seguir habiendo una lista paralela en routes.py. Además quita hoy mismo el modo de falla de la desincronización.
(c) tercero. Aquí está el mayor beneficio de riesgo: es el olor con el modo de falla silencioso. El trabajo es un registro en payments/ y quitar el if/elif del checkout. Ojo con el orden: el registro se introduce primero en paralelo al if/elif, se verifica que produce lo mismo, y después se quita el condicional. La lección 6 desarrolla esa secuencia.
(a) al final, y por partes. Es el más grande, el más riesgoso y el que menos conviene abordar de frente. Y hay una razón adicional para dejarlo al final: buena parte de (a) se resuelve como consecuencia de (c). Sacar el bloque de cobro del checkout le quita una de sus cuatro responsabilidades y unas veinte líneas. Después de eso, la conversación sobre si vale la pena extraer también el precio y los avisos se tiene con un archivo más chico y con un equipo que ya vio funcionar dos refactors.
Lo que hay que decirle al compañero además del orden. Que ninguno de los cuatro es urgente en el sentido de "hay un bug en producción", y que por lo tanto el orden importa menos que el ritmo: uno por sprint, cada uno con su prueba, cada uno mergeado antes de empezar el siguiente. Cuatro refactors abiertos al mismo tiempo sobre el mismo módulo es una receta para conflictos y para revertir todo.
Por qué funciona: priorizar es la parte del oficio que no aparece en ningún catálogo de olores. Un diagnóstico correcto que se ataca en el orden equivocado —empezando por el God object, que es lo más visible— suele terminar en un refactor grande, riesgoso, abandonado a la mitad. Empezar por lo barato y dejar que los arreglos se ayuden entre sí es lo que hace que el trabajo termine.
Resumen y siguiente paso
En esta lección abriste con lupa los tres olores que más aparecen y más cuestan.
God object: una clase o archivo que concentra tantas responsabilidades que todo pasa por ahí. Lo define la variedad de razones de cambio, no el tamaño. Se detecta contando imports, contando razones de cambio, mirando el historial (checkout.py en trece de los últimos doscientos commits) y preguntando por conflictos de merge. Se resuelve extrayendo colaboradores por razón de cambio —Strategy para el precio, Factory para el cobro, un notify para los avisos— con la advertencia de que cortar por tamaño en vez de por responsabilidad produce cinco God objects chiquitos.
Feature envy: un método que usa más datos de otra clase que de la propia. Se detecta contando accesos self. contra accesos al parámetro. Se resuelve mudando el método a donde viven los datos, con dos matices: si la envidia está repartida entre dos clases, lo que falta es un concepto, no una mudanza; y si el objeto envidiado es un DTO o algo externo, no es envidia. El beneficio real no es el orden: es que al mudar aparecen reglas del dominio que estaban disueltas, como is_expired_cash_payment.
Shotgun surgery: un cambio conceptual que obliga a tocar muchos archivos. Es el opuesto simétrico de divergent change y es el más caro de los tres porque su modo de falla es silencioso. Se detecta con la prueba del cambio hipotético y con el historial de archivos que cambian juntos. Se resuelve dándole al concepto disperso un solo lugar donde vivir —un registro, una Factory—, con la advertencia de que no toda dispersión es accidental.
Y viste cómo se encadenan: un God object produce shotgun surgery en los conceptos que absorbió, y feature envy es la versión local del mismo problema. De ahí el orden de ataque: envidia primero (barato y revelador), dispersión después (quita el riesgo silencioso), God object al final y por partes.
Antes de avanzar deberías poder: detectar los tres con evidencia contable y no con impresiones; nombrar un falso positivo de cada uno; y explicar por qué shotgun surgery es el más caro aunque God object sea el más visible.
Lo que todavía no tienes es cómo decirlo. Tienes el diagnóstico, tienes la evidencia y tienes la dirección; y aun así, mal escrito, todo eso puede terminar en un comentario que el autor no sabe cómo atender o que lo pone a la defensiva. La lección 4 es la lección técnica del módulo: la fórmula de cinco partes —nombre, ubicación, consecuencia, dirección y peso— con ejemplos antes y después de comentarios reales, y el criterio para saber cuándo no escribir uno.
Recursos
- Refactoring, capítulo 3 — Bad Smells in Code (Martin Fowler) — la fuente de Large Class, Feature Envy, Shotgun Surgery y Divergent Change, con las refactorizaciones asociadas a cada uno.
- Refactoring Guru — Move Method — la refactorización que corrige feature envy, con el paso a paso y los casos donde no conviene aplicarla.
- Your Code as a Crime Scene, 2ª edición (Adam Tornhill) — el libro de donde salen las técnicas de historial de esta lección: puntos calientes por frecuencia de cambio y acoplamiento lógico entre archivos que cambian juntos.
- Code Maat — la herramienta libre del mismo autor que automatiza esos análisis sobre un repositorio de Git. Útil cuando el
git loga mano se queda corto.