Módulo 7: Patrones como vocabulario de revisión
2. El vocabulario de lo que está mal: code smells
Descripción
Al terminar esta lección vas a tener tres cosas. Primero, una definición precisa de qué es un code smell —un olor— y, sobre todo, de qué no es: no es un error, no es una opinión, y no es una orden de refactorizar. Segundo, vas a entender la distinción que hace toda la diferencia práctica: un olor se investiga, no se corrige de reflejo. Esa frase suena a matiz y es en realidad la línea que separa a alguien que diagnostica de alguien que reparte refactors. Y tercero, vas a salir con el catálogo corto y útil —los doce olores que de verdad aparecen— con su ejemplo concreto en Boletia, listo para usar en la lección siguiente.
Esto importa porque el vocabulario de lo que está mal tiene un riesgo que el vocabulario de los patrones no tiene. Cuando aprendes "Strategy", lo peor que puede pasar es que la apliques donde no hacía falta —malo, pero visible y reversible—. Cuando aprendes "God object", lo peor que puede pasar es que empieces a repartir la palabra como si fuera un veredicto, sobre código que quizá esté perfectamente bien. Los olores son señales probabilísticas, no sentencias, y quien los usa como sentencias hace más daño que quien no los conoce. Por eso esta lección dedica tanto espacio a la palabra probablemente como al catálogo mismo.
Y hay una razón positiva, más importante que la defensiva. Con el catálogo en la cabeza, leer código ajeno cambia de textura. Sin él, abres un archivo de trescientas líneas y sientes una incomodidad difusa que no sabes dónde poner. Con él, abres el mismo archivo y ves cosas contables: "aquí hay un switch sobre kind que ya vi en otros dos archivos", "este método toca siete atributos de Order y ninguno propio", "esta clase importa catorce módulos". La incomodidad difusa se convierte en una lista. Y una lista se puede priorizar, discutir y decidir.
Conexión con el módulo: la lección 1 mostró la diferencia entre un comentario vago y uno nombrado, pero todavía no te dio los nombres. Esta lección instala el concepto que los ordena a todos —el olor como señal— y entrega el catálogo. La lección 3 toma los tres más frecuentes y caros y los abre con lupa, uno por uno. La lección 4 convierte el nombre en un comentario accionable. Y la lección 5 sube un nivel: de los olores, que son señales locales en el código, a los anti-patrones, que son decisiones de diseño con nombre.
Lo que hace un olor en una cocina
Estás en tu casa y hueles a gas.
Hazte la pregunta importante: ¿el olor es el problema? No. El olor no te hace daño; es la molécula que le agregan al gas —tiol, un compuesto que huele espantoso a propósito— justamente para que lo notes, porque el gas natural no huele a nada. El problema es la fuga, y la fuga es invisible.
Ahora fíjate en lo que haces con ese olor. No cambias la estufa. Vas, revisas si quedó una hornilla abierta, hueles cerca de la manguera, abres la ventana. Investigas. Y hay tres desenlaces posibles, los tres normales:
- Encuentras la fuga. El olor te salvó. La reparación es lo que arregla el problema, no el olor.
- Encuentras algo distinto. Alguien encendió el bóiler y se apagó la llama. No era una fuga, pero sí era algo que valía la pena mirar.
- No encuentras nada. Era el vecino, o era la basura orgánica. El olor fue un falso positivo y no pasa nada: el costo de investigar era bajo y el costo de ignorarlo hubiera sido alto.
Un code smell funciona exactamente igual. Es un rasgo observable en el código que suele acompañar a un problema de estructura, pero que no es el problema. El código con olor normalmente funciona: pasa las pruebas, atiende usuarios, factura dinero. El olor no rompe nada hoy. Lo que indica es que probablemente hay algo mal acomodado que va a costar caro la próxima vez que alguien tenga que cambiar esa zona.
Y como el olor a gas, tiene falsos positivos legítimos. Un método de ochenta líneas normalmente es un problema; un método de ochenta líneas que es una tabla de conversión de códigos de país, plana y sin ramas, no lo es. Alguien que corrige de reflejo lo parte en cuatro métodos de veinte y deja el código peor. Alguien que investiga lo mira, entiende qué es, y sigue con su día.
Hay un tercer paralelo con el gas que quiero que te lleves, porque es el que más cuesta aceptar: el olor no te dice el tamaño de la fuga. Huele igual una fuga minúscula en una conexión que una grave. En código pasa lo mismo: un if/elif de tres ramas y uno de veinte huelen al mismo olor —switch repetido— y tienen consecuencias completamente distintas. Detectar el olor es el primer paso; medir su costo es un paso aparte, y es el que convierte un hallazgo en una prioridad. La lección 4 va a insistir en eso cuando hablemos de la parte "consecuencia" de un comentario.
Qué es exactamente un olor (y sus tres partes)
Un code smell es un rasgo superficial y observable del código que, con frecuencia estadística alta, indica un problema de estructura más profundo.
Esa definición tiene tres palabras cargadas y vale la pena abrirlas una por una, porque cada una descarta un malentendido común.
Superficial. El olor se ve sin entender el dominio. No necesitas saber qué es un boleto de cortesía para notar que calculate_price tiene un if por cada valor de un campo, ni para contar que ReconciliationReport.notify_finance usa siete atributos de Order y ninguno propio. Esto es lo que hace al vocabulario enseñable y compartible: dos personas que miran el mismo archivo llegan al mismo hallazgo aunque una lleve tres años en el equipo y la otra tres semanas. Compáralo con "este código está mal pensado", que requiere conocer el negocio y no se puede verificar.
Observable. Un olor se puede señalar con el dedo y, casi siempre, contar. No es "siento que esta clase hace mucho": es "esta clase tiene catorce métodos, importa once módulos y la tocaron trece de los últimos veinte PRs". Esa propiedad —que sea contable— es lo que lo saca del terreno del gusto. Cuando dos personas discuten sobre si algo es feo, la discusión no termina. Cuando discuten sobre si trece de veinte PRs tocaron el mismo archivo, la discusión termina en dos minutos: se abre el historial y se cuenta.
Con frecuencia alta, no siempre. Aquí está el corazón de la lección. Un olor es una correlación, no una implicación. Dice: "en la mayoría de los casos donde se ve esto, hay un problema debajo". No dice: "aquí hay un problema". Esa distinción es la que autoriza —y obliga— a investigar antes de actuar.
Ahora, la anatomía práctica. Cada vez que uses un olor en una revisión, vas a estar manejando tres piezas, y conviene tenerlas separadas en la cabeza porque la lección 4 las va a pedir explícitamente:
| Pieza | Qué es | Ejemplo en Boletia |
|---|---|---|
| El síntoma | Lo que se ve y se cuenta. Verificable por cualquiera | calculate_price tiene un if por cada valor de ticket.kind, y la misma pregunta se contesta otra vez en tickets/transfer.py y en refunds/policy.py |
| La hipótesis | Qué problema de estructura suele haber debajo | El comportamiento que depende del tipo de boleto está repartido en tres archivos en vez de vivir junto al tipo |
| El costo | Qué va a doler, cuándo y a quién | Agregar un quinto tipo de boleto obliga a encontrar los tres lugares. El que se olvide de uno produce un bug silencioso: el boleto nuevo se comporta como general en reembolsos |
Fíjate en que el síntoma es un hecho, la hipótesis es una interpretación y el costo es una predicción. Las tres pueden discutirse, pero se discuten distinto. Sobre el síntoma se discute contando. Sobre la hipótesis se discute con argumentos de diseño. Sobre el costo se discute con la experiencia del equipo y con lo que el roadmap tenga previsto. Mezclarlas es la fuente de la mitad de las discusiones que no llegan a nada en una revisión.
Ejemplo trabajado: un olor investigado, no corregido
Vamos a hacer el ciclo completo sobre un caso de Boletia, y vamos a hacerlo con el desenlace que casi nunca se muestra en un curso: uno donde parte del hallazgo se corrige y parte se decide dejar.
Abres pricing/calculator.py porque tienes que revisar un PR que le agrega el tipo de boleto student.
# Archivo: pricing/calculator.py
def calculate_price(ticket, order_date):
if ticket.kind == "general":
return ticket.base_price
elif ticket.kind == "vip":
return ticket.base_price * 1.40
elif ticket.kind == "early_bird":
cutoff = get_early_bird_cutoff(ticket.event_id)
return ticket.base_price * 0.75 if order_date < cutoff else ticket.base_price
elif ticket.kind == "courtesy":
if courtesy_count(ticket.event_id) > COURTESY_LIMIT:
raise CourtesyLimitExceeded(ticket.event_id)
return 0.0
else:
raise ValueError(f"Tipo de boleto desconocido: {ticket.kind}")
Paso 1 — el síntoma, dicho sin interpretación. Hay un if/elif que ramifica sobre el valor de un campo de texto libre, con una rama por valor posible. El olor tiene nombre: switch sobre un tipo (Fowler lo llama switch statements, y su primo cercano es primitive obsession, porque kind es un str y no un tipo propio).
Nota lo que no dije: no dije "esto está mal". Un if de cuatro ramas en un solo lugar es perfectamente defendible. Lo que dije es que se ve la señal.
Paso 2 — la investigación. Aquí es donde se gana o se pierde el diagnóstico, y donde la mayoría se salta el paso. La pregunta que hay que contestar es: ¿esta misma pregunta se contesta en algún otro lado? Porque un switch en un solo lugar es una función; el mismo switch en cinco lugares es un problema de estructura.
# Buscamos todos los lugares del código que preguntan por el tipo de boleto.
# No es sofisticado y no hace falta que lo sea: el objetivo es contar.
$ grep -rn "\.kind" --include="*.py" .
Y sale esto:
pricing/calculator.py:4 if ticket.kind == "general":
pricing/calculator.py:6 elif ticket.kind == "vip":
pricing/calculator.py:8 elif ticket.kind == "early_bird":
pricing/calculator.py:11 elif ticket.kind == "courtesy":
tickets/transfer.py:22 if ticket.kind in ("courtesy",):
refunds/policy.py:15 if ticket.kind == "early_bird":
reports/attendees.py:41 "type": TICKET_LABELS.get(ticket.kind, ticket.kind),
api/routes.py:88 if payload["kind"] not in ALLOWED_KINDS:
Paso 3 — leer cada aparición. Esto es lo que separa investigar de contar. Cinco archivos aparecen, pero no todos dicen lo mismo:
pricing/calculator.py— decide comportamiento según el tipo. Cuatro ramas, lógica real en cada una.tickets/transfer.py— decide comportamiento: las cortesías no se transfieren.refunds/policy.py— decide comportamiento: los early-bird tienen tres días de ventana de reembolso, los demás catorce.reports/attendees.py— no decide nada: traduce el valor a una etiqueta para mostrarla. Es una tabla de presentación.api/routes.py— no decide nada: valida que el valor de entrada esté en la lista permitida.
Aquí aparece el hallazgo real, y no es el que se veía al principio. Tres archivos toman decisiones de comportamiento según el tipo de boleto, y ninguno de los tres sabe de los otros dos. Los otros dos usos son inofensivos: presentación y validación de entrada no son comportamiento del dominio.
Paso 4 — el costo, en escenario concreto. El PR que estás revisando agrega student. Recorramos qué tiene que pasar para que ese tipo funcione completo:
pricing/calculator.py— nueva rama con el descuento. El PR lo hace.api/routes.py— agregar"student"aALLOWED_KINDS. El PR lo hace.reports/attendees.py— agregar la etiqueta. El PR lo hace.refunds/policy.py— decidir la ventana de reembolso de un boleto de estudiante. El PR no lo toca.tickets/transfer.py— decidir si un boleto de estudiante se transfiere. El PR no lo toca.
Los puntos 4 y 5 no fallan. No hay excepción, no hay prueba en rojo, nada se rompe. El boleto de estudiante simplemente cae en el else de cada uno y se comporta como un boleto general: catorce días de reembolso y transferible sin restricción. Puede que eso sea justo lo que el negocio quiere. Puede que no. Nadie lo decidió: se decidió solo, por omisión.
Ese es el costo del olor, dicho en su forma más útil: no es que el código sea feo, es que el sistema toma decisiones que nadie tomó.
Paso 5 — la decisión, que es lo que casi nunca se enseña. Tienes el diagnóstico. Ahora, ¿qué haces?
La respuesta de reflejo sería: "esto pide una Strategy, hay que mover el comportamiento por tipo a clases TicketKind". Y podría ser correcta a mediano plazo. Pero el módulo 2 dejó una pregunta de bolsillo que aplica exactamente aquí: ¿el rediseño se gana su lugar hoy? Vamos a pensarlo en voz alta.
A favor de refactorizar: son tres lugares reales, ya se contestaron distinto, y el modo de falla es silencioso —que es el peor—. En contra: el PR que estás revisando es de una funcionalidad, no de un rediseño; pedirle a esa persona que reestructure el modelo de tipos de boleto para poder agregar student es multiplicar por diez el tamaño del cambio, y la persona que lo escribió probablemente lleva dos semanas en el equipo.
La salida sensata es partir el hallazgo en dos comentarios de peso distinto:
Bloqueante —
refunds/policy.pyytickets/transfer.py(fuera del diff, pero consecuencia directa de este cambio): estos dos archivos también deciden segúnticket.kind, y con este PRstudentva a caer en suelse. Eso significa catorce días de reembolso y transferencia libre, decidido por omisión. ¿Es lo que quiere producto? Si sí, agreguemos la rama explícita igual, para que la decisión quede escrita. Si no, hay que cambiarla aquí.
Nota, no bloqueante — el patrón de fondo:
ticket.kindes unstrsobre el que hoy ramifican tres archivos independientes (pricing,refunds,transfer). Es un switch repetido con primitive obsession debajo, y el modo de falla es el que acabamos de ver: un tipo nuevo se comporta comogeneralsin que nadie lo decida. Cuando entre el sexto tipo de boleto, vale la pena mover el comportamiento por tipo a un solo lugar (una Strategy porkind, con las tres preguntas —precio, ventana de reembolso, transferible— como métodos). No es trabajo para este PR; lo dejo escrito para que exista.
Qué esperar de este recorrido. Lo primero: el hallazgo cambió durante la investigación. Empezaste viendo "un if largo en calculator.py" y terminaste con algo completamente distinto y mucho más grave: "hay tres archivos que deciden por tipo y no se conocen entre sí, y este PR va a dejar dos decisiones tomadas por omisión". Si hubieras corregido de reflejo —partir el if en funcioncitas, o meter una Strategy ahí mismo— habrías arreglado la estética de calculator.py y el bug de refunds habría entrado igual. Investigar no es lentitud: es lo que hace que apuntes al problema y no al síntoma.
Lo segundo: el desenlace tiene dos pesos distintos. Una parte es bloqueante y es chiquita —dos ramas explícitas—; la otra es estructural y no bloquea nada. Meterlas en el mismo comentario habría producido el efecto clásico: el autor lee un párrafo sobre Strategy, siente que le están pidiendo rediseñar el sistema para agregar un tipo de boleto, y se atasca. Separarlas le permite hacer lo urgente hoy y dejar registrado lo importante.
Lo tercero, y es el que más cuesta: el olor se investigó y la mayor parte se decidió dejar. El código de calculator.py sale de esta revisión exactamente igual que entró, con una rama más. Y eso está bien. La lección 1 lo decía y aquí se ve funcionando: la diferencia entre un olor ignorado y un olor decidido es enorme, aunque el código se vea igual.
El catálogo corto
Fowler catalogó más de veinte olores; refactoring.guru lista unos veintitantos agrupados en cinco familias. La lista completa es una referencia útil, no un plan de estudio. Estos doce son los que de verdad aparecen en revisiones de código real, y los que vas a poder usar sin sentir que estás forzando la etiqueta.
Los agrupo por la pregunta que responde cada uno, porque así se recuerdan mejor que en orden alfabético.
Olores de tamaño: "esto es demasiado grande"
Long method — el método largo. Una función que hace tantas cosas que para entenderla hay que leerla completa y llevar la cuenta mentalmente. La señal no es el número de líneas sino el número de niveles de abstracción mezclados: si una misma función calcula un total, arma un cliente HTTP y formatea un mensaje, está saltando entre tres alturas distintas. En Boletia: checkout(), que orquesta precio, asientos, cobro y avisos en un solo cuerpo. Falso positivo típico: una tabla de datos larga y plana, o una función de configuración que solo asigna valores.
Large class / God object — la clase que lo sabe todo. Muchas responsabilidades, muchos atributos, muchos colaboradores. Es el olor de tamaño más caro porque atrae más código: como ya está todo ahí, meter una cosa más siempre parece lo más fácil. En Boletia: checkout/checkout.py. La lección 3 lo abre con lupa.
Long parameter list — la lista de parámetros larga. Una función que pide seis, ocho, diez argumentos. Suele indicar que varios de esos parámetros son en realidad un concepto que no tiene nombre todavía. Si send_confirmation(email, name, order_id, total, event_name, venue, starts_at) te incomoda, es porque los últimos tres son un Event y los del medio son una Order. Su primo es data clump: el mismo grupo de tres o cuatro valores que viaja junto por todo el sistema.
Olores de cambio: "esto va a doler cuando lo toques"
Divergent change — el archivo que cambia por muchas razones. Un archivo que se modifica por motivos que no tienen nada que ver entre sí: hoy porque cambió una tarifa, mañana porque cambió el formato de un correo, pasado porque cambió el proveedor de pago. Señal de que dentro conviven responsabilidades que deberían estar separadas. En Boletia: checkout.py otra vez.
Shotgun surgery — el cambio que se dispersa. Lo contrario exacto del anterior: un solo cambio conceptual obliga a tocar muchos archivos. En Boletia: agregar un proveedor de pago toca cinco lugares. La lección 3 lo abre con lupa.
Vale la pena que veas estos dos juntos, porque son simétricos y la simetría ayuda a recordarlos:
| Un cambio | Muchos cambios | |
|---|---|---|
| Un archivo | Lo normal | Divergent change |
| Muchos archivos | Shotgun surgery | Lo normal en un sistema grande |
Duplicated code — el código repetido. El más conocido y el peor entendido. Repetir tres líneas no siempre es un problema; repetir una decisión casi siempre lo es. La pregunta útil no es "¿se parecen?" sino "¿si cambia una, tienen que cambiar todas?". En Boletia: los tres exportadores de reports/ repiten el mismo esqueleto —pedir, ordenar, formatear, escribir— y solo cambia un paso. Eso apunta a Template Method (módulo 3). Cuidado con el falso positivo: dos trozos que hoy se ven iguales pero cambian por razones distintas no son duplicación, y unificarlos es acoplarlos. El módulo 2 dedicó media lección a esa trampa.
Olores de responsabilidad: "esto no vive donde debería"
Feature envy — la envidia de datos. Un método que usa más datos de otra clase que de la propia. En Boletia: un notify_finance(order) que toca siete atributos de Order y ninguno de su propia clase. La lección 3 lo abre con lupa.
Message chain — la cadena de mensajes. Código que navega por la estructura interna de otros objetos: order.customer.event.organizer.email. El problema no es la longitud, es que quien escribió esa línea depende de cuatro estructuras para conseguir un dato. Si cualquiera de los cuatro eslabones cambia, esta línea se rompe. La regla de bolsillo que lo describe se conoce como la ley de Demeter: habla con tus amigos, no con los amigos de tus amigos.
Middle man — el intermediario vacío. Una clase cuyos métodos solo delegan en otra sin agregar nada. Es el olor inverso al de la indirección faltante, y es la forma en que un patrón bienintencionado se convierte en peso muerto. Cuidado con el falso positivo: un Adapter (módulo 5) delega casi todo a propósito, y ahí la delegación es el valor —está traduciendo entre dos interfaces—. La pregunta que separa uno de otro: ¿este intermediario cambia algo, o solo pasa la pelota?
Olores de tipos: "aquí falta un concepto"
Primitive obsession — la obsesión con los primitivos. Conceptos del negocio que viajan como str, int o dict suelto en vez de tener un tipo propio. En Boletia hay tres ejemplares de libro: Ticket.kind es un str, Order.provider es un str, y Order.status es un str. Los tres son en realidad conjuntos cerrados de valores con comportamiento asociado, y como son texto libre, el compilador —o el editor, o la prueba— no puede ayudarte: un "vip " con espacio al final cae en el else sin decir nada.
Switch repetido sobre un tipo. El que acabamos de investigar. La señal es el mismo if sobre el mismo campo, contestado en más de un archivo. Casi siempre viene emparejado con primitive obsession, porque el switch existe precisamente porque el tipo no tiene comportamiento propio.
Olores de exceso: "esto sobra"
Speculative generality — la generalidad especulativa. Estructura de extensión construida para necesidades que nunca llegaron: una interfaz con un implementador, un parámetro que siempre recibe el mismo valor, una clase abstracta con un solo hijo, un mecanismo de plugins con un plugin. En Boletia: plugins/, que el módulo 2 desmontó paso a paso en su lección 6. Este es el único olor del catálogo cuyo tratamiento es quitar, no agregar.
Comments as deodorant — el comentario que tapa. Un comentario largo que explica qué hace un bloque enigmático. No es que comentar esté mal; es que un comentario que explica qué hace un fragmento suele ser una señal de que ese fragmento pedía un nombre. # Aquí calculamos la comisión según el monto y el proveedor casi siempre significa que faltaba una función llamada calculate_commission(amount, provider). La distinción: un comentario que explica por qué es valioso y no huele a nada; uno que explica qué suele estar sustituyendo un nombre.
La tabla de bolsillo
| Olor | Señal en una línea | En Boletia |
|---|---|---|
| Long method | Una función con varios niveles de abstracción mezclados | checkout() |
| God object | Muchas responsabilidades, muchos colaboradores, todos lo tocan | checkout/checkout.py |
| Long parameter list | Seis o más argumentos; varios forman un concepto sin nombre | send_confirmation(...) |
| Divergent change | Un archivo que cambia por razones sin relación | checkout/checkout.py |
| Shotgun surgery | Un cambio conceptual que toca muchos archivos | Agregar un proveedor de pago: 5 lugares |
| Duplicated code | Si cambia una copia, tienen que cambiar todas | Los tres exportadores de reports/ |
| Feature envy | Un método usa más datos ajenos que propios | notify_finance(order) |
| Message chain | a.b.c.d — se camina por la estructura de otros | order.customer.event.organizer.email |
| Middle man | Delega todo y no agrega nada | Un manager que solo reenvía |
| Primitive obsession | Un concepto del negocio viaja como str o int | Ticket.kind, Order.provider, Order.status |
| Switch repetido | El mismo if sobre el mismo campo, en varios archivos | ticket.kind en pricing, refunds, transfer |
| Speculative generality | Extensión construida para algo que no llegó | plugins/ |
Imprímela mentalmente, no la memorices. La forma de aprender este catálogo no es repasarlo: es abrir un archivo cualquiera de tu trabajo, recorrer la tabla, y anotar cuáles ves. La primera vez vas a ver cuatro donde hay uno. La quinta vez vas a ver el que importa.
Por qué un olor se investiga y no se corrige de reflejo
Ya viste la investigación en acción. Ahora quiero dejar la regla explícita, porque es la idea que más se pierde cuando alguien aprende este vocabulario.
Un olor detectado no autoriza un cambio. Autoriza una pregunta. Y la pregunta tiene tres partes que conviene hacerse en orden:
Primera: ¿es real? Es decir, ¿el síntoma es lo que parece? Los falsos positivos existen y son frecuentes. El método largo que resultó ser una tabla. La duplicación que resultó ser dos cosas que se parecen hoy y cambian por razones distintas. El intermediario que resultó ser un Adapter haciendo su trabajo. Esta primera pregunta se responde leyendo, y es la que más gente se salta.
Segunda: ¿cuánto cuesta? Un olor sin costo no es un problema, es una curiosidad. Y el costo se mide en trabajo futuro concreto, no en principios. La forma de estimarlo que mejor funciona es el escenario de cambio: elige un cambio plausible que el equipo vaya a necesitar en los próximos meses —agregar un proveedor, agregar un tipo de boleto, cambiar el formato del correo— y recorre qué archivos hay que tocar y qué puede salir mal. Si el recorrido es corto y sin sorpresas, el olor es cosmético. Si el recorrido pasa por cinco archivos y uno de los modos de falla es silencioso, ahí tienes tu caso.
Hay una segunda fuente de evidencia, gratuita y muy infravalorada: el historial. Si un archivo aparece en trece de los últimos veinte cambios, eso no es una opinión sobre su diseño; es un hecho sobre cómo se comporta el equipo alrededor de él.
# Los archivos que más se tocaron en los últimos 200 commits.
# No prueba que estén mal diseñados, pero sí dice dónde vive el dolor.
$ git log --format=format: --name-only -n 200 | sort | uniq -c | sort -rg | head -10
Tercera: ¿ahora? Esta es la pregunta de criterio, y es la que más se parece al módulo 2. Un olor real y caro puede seguir siendo la cosa equivocada para arreglar hoy, por tres razones legítimas: porque el PR en el que estás es de otra cosa; porque falta información —van a llegar dos tipos de boleto más y el diseño correcto todavía no se ve—; o porque hay algo más caro pendiente. Decidir "todavía no" con el diagnóstico escrito es una decisión de ingeniería perfectamente respetable. Lo que no es respetable es no haber mirado.
Una nota sobre esta tercera pregunta que conecta con toda la guía. La respuesta "sí, ahora, y con un patrón" es una de varias, y no la más frecuente. Muchas veces el arreglo correcto no involucra ningún patrón: extraer una función, renombrar una variable, mover un método a otra clase. Los patrones son la respuesta cuando el problema es de estructura que varía; para todo lo demás hay refactorizaciones simples con nombres poco glamorosos. Confundir "detecté un olor" con "hay que meter un patrón" es exactamente el reflejo que este módulo intenta desactivar.
Errores comunes
Tratar el olor como el problema (conceptual). Qué pasa: alguien detecta un método de ochenta líneas y lo parte en cuatro métodos de veinte, con nombres tipo _step_one, _step_two. El olor "método largo" desapareció y no se arregló nada: ahora hay que leer cuatro funciones en vez de una, y ninguna tiene sentido por sí sola. Por qué pasa: el olor es lo visible y lo medible, y hacerlo desaparecer produce una sensación inmediata de progreso. Cómo detectarlo: si después de tu refactor no puedes nombrar qué problema de estructura resolviste —solo qué señal apagaste—, no resolviste nada. La prueba más directa: si los métodos nuevos no tienen nombres que signifiquen algo en el dominio (calculate_service_fee, assign_seats) sino nombres de posición (_part_two), partiste por tamaño y no por responsabilidad. Cómo corregirlo: antes de tocar, escribe en una frase la hipótesis —qué está mal acomodado— y el costo —qué va a doler—. Si no puedes escribirlas, todavía no investigaste lo suficiente.
Repartir etiquetas sin haber leído (de método). Qué pasa: alguien recorre un diff con la tabla del catálogo al lado y va marcando: "long method", "primitive obsession", "feature envy". Ocho comentarios en cuatro minutos. La mitad son falsos positivos y el autor lo sabe, así que empieza a descontar todos tus comentarios, incluidos los buenos. Por qué pasa: el catálogo se puede aplicar superficialmente, y eso es justamente lo que lo hace peligroso. Reconocer la forma de un olor no requiere entender el código; distinguir un olor real de un falso positivo sí. Cómo detectarlo: si tus comentarios no contienen ninguna evidencia contable —un número, una lista de archivos, un escenario— son etiquetas. Cómo corregirlo: la regla del ejemplo trabajado. Un olor detectado exige una búsqueda antes del comentario: grep del campo, revisar cuántos usos hay, leer si deciden o solo muestran. Esa búsqueda toma dos minutos y convierte una etiqueta en un diagnóstico.
Creer que el catálogo es una lista de prohibiciones (de criterio). Qué pasa: alguien sale de una lección como esta convencido de que un if sobre un tipo es malo, que los métodos largos son malos, que la duplicación es mala, y empieza a escribir código evitando los olores en vez de resolviendo problemas. El resultado suele ser peor que el código que evitó: jerarquías de clases para dos casos, funciones de tres líneas encadenadas, abstracciones prematuras por todas partes. Por qué pasa: es más fácil seguir una lista de prohibiciones que ejercer criterio, y una lista da la sensación tranquilizadora de estar haciendo lo correcto. Cómo detectarlo: si estás introduciendo indirección antes de tener un segundo caso real, estás programando contra la lista. Cómo corregirlo: el módulo 2 completo, y la regla de tres en particular. Los olores son herramientas para leer código existente, no reglas para escribir código nuevo. Escribir código nuevo evitando olores imaginarios produce el olor que cierra este catálogo: speculative generality.
Ejercicios
Ejercicio 1 — Separa el síntoma de la hipótesis y del costo. Aquí hay tres observaciones sobre Boletia, escritas como se dicen normalmente. Para cada una, sepárala en las tres piezas: qué es el síntoma observable, cuál es la hipótesis sobre la estructura, y cuál es el costo concreto. Si alguna de las tres piezas falta en el original, complétala tú.
(a) "checkout.py es un desastre."
(b) "Los tres exportadores de reports/ están copiados."
(c) "order.provider siendo un string me da mala espina."
Ver solución
(a) Síntoma: el original no tiene ninguno; "es un desastre" es una valoración. Un síntoma real sería: checkout() tiene unas 300 líneas, orquesta cuatro cosas sin relación entre sí (precio, asientos, cobro, avisos), importa once módulos y aparece en trece de los últimos veinte commits. Hipótesis: conviven varias responsabilidades que deberían estar separadas —es un God object con divergent change—. Costo: cualquier cambio en cualquiera de las cuatro áreas obliga a tocar el archivo más delicado del sistema, y dos personas trabajando en cosas distintas chocan en el mismo archivo. Fíjate en cuánto trabajo hizo falta para convertir la frase original en algo discutible: eso es la diferencia entre una queja y un diagnóstico.
(b) Síntoma: los tres exportadores ejecutan la misma secuencia de cuatro pasos en el mismo orden (fetch → sort → format → write) y solo difieren en el tercero. Es contable: se pueden poner los tres archivos lado a lado. Hipótesis: el esqueleto del algoritmo está duplicado; solo el paso variable debería estar en cada clase. Apunta a Template Method. Costo: un cambio en el esqueleto —paginar la consulta, cambiar el criterio de orden, agregar una columna al encabezado— hay que hacerlo tres veces, y el que se olvide de una produce una inconsistencia que solo se nota en producción. Nota que el original tenía el síntoma pero le faltaban hipótesis y costo, y sin costo no hay caso.
(c) Síntoma: Order.provider es un str sin restricción, y hay al menos dos listas blancas de valores permitidos en archivos distintos (api/routes.py y el if/elif de checkout.py). Hipótesis: primitive obsession — un concepto cerrado del dominio viajando como texto libre, con el comportamiento asociado repartido. Costo: un valor con error de tipeo o de caja ("Stripe", "stripe ") cae en el else y produce un ValueError en el corazón del checkout; y las dos listas se pueden desincronizar, de modo que la API acepte un proveedor que el checkout no sabe cobrar, o al revés. "Mala espina" es una intuición legítima, pero no se puede discutir; esto sí.
Por qué funciona: las tres piezas son exactamente lo que la lección 4 va a pedirte para escribir un comentario accionable. Practicar la separación aquí, sobre observaciones que ya tenías, hace que allá la fórmula te resulte natural en vez de burocrática.
Ejercicio 2 — Investiga antes de decidir. Te toca revisar este archivo nuevo de Boletia. Nombra los olores que ves, y para cada uno di qué buscarías antes de escribir un comentario. No propongas todavía ninguna solución.
# Archivo: notifications/manager.py
class NotificationManager:
def __init__(self, db, mailer, sms_client, push_client, analytics, settings):
self.db = db
self.mailer = mailer
self.sms_client = sms_client
self.push_client = push_client
self.analytics = analytics
self.settings = settings
def send_order_confirmation(self, order):
customer = self.db.get_customer(order.customer_id)
event = self.db.get_event(self.db.get_ticket(order.ticket_ids[0]).event_id)
body = (
f"Hola {customer.name}, tu compra por ${order.total} "
f"para {event.name} en {event.venue} el {event.starts_at} está lista. "
f"Orden #{order.id}, {len(order.ticket_ids)} boletos."
)
if order.provider == "cash":
body += " Recuerda pagar en tienda antes de 48 horas."
self.mailer.send(customer.email, "Tu compra en Boletia", body)
if customer.phone:
self.sms_client.send(customer.phone, body[:140])
if customer.push_token:
self.push_client.send(customer.push_token, body[:80])
self.analytics.track("confirmation_sent", order_id=order.id)
Ver solución
Cuatro olores, y para cada uno una búsqueda concreta.
Long parameter list en el constructor — seis dependencias. Qué buscar antes de comentar: cuántos de los seis usa cada método de la clase. Si send_order_confirmation usa cinco y otro método usa dos distintos, no es una lista larga: son dos clases juntas, y el olor real es God object. Si todos los métodos usan casi todo, la lista larga es honesta y el comentario sería otro.
Feature envy hacia Order, Customer y Event — el método arma un texto usando puros datos ajenos: customer.name, order.total, event.name, event.venue, event.starts_at, order.id, order.ticket_ids. Siete accesos externos, cero atributos propios (salvo los colaboradores inyectados). Qué buscar: si existe ya algún build_confirmation(order) en otro lado del sistema. En Boletia sí existe, y lo llama checkout() — así que además hay duplicación de la construcción del mensaje.
Switch sobre order.provider — el if order.provider == "cash" metido en medio del armado del texto. Qué buscar: grep -rn "provider ==" . para ver cuántos archivos deciden por proveedor. Si son varios, esto es el mismo shotgun surgery que ya conocemos, ahora asomándose en notificaciones.
Message chain — self.db.get_event(self.db.get_ticket(order.ticket_ids[0]).event_id). No es la forma clásica a.b.c.d, pero el problema es idéntico: esta línea depende de que una orden tenga al menos un boleto, de que el boleto tenga event_id, y de dos llamadas al repositorio para conseguir un dato. Qué buscar: si Order tiene o podría tener una forma directa de dar su evento; y qué pasa hoy si ticket_ids viene vacío —muy probablemente un IndexError dentro del envío de un correo, que es un lugar espantoso para explotar—.
Nota lo que no aparece en esta lista: ninguna propuesta de solución. El ejercicio pedía investigar, y en una revisión real la investigación cambia el comentario. Si la búsqueda de build_confirmation no encuentra nada, el comentario sobre feature envy es una nota menor. Si encuentra que el mismo texto se arma en tres lugares con formatos ligeramente distintos, el comentario pasa a ser sobre duplicación y probablemente sea bloqueante.
Por qué funciona: el hábito que quiero instalar es que entre "veo algo" y "escribo algo" hay un paso, y ese paso normalmente es un grep de dos minutos. Ese paso es lo que convierte a alguien que reparte etiquetas en alguien cuyos comentarios el equipo lee con atención.
Ejercicio 3 — Encuentra el falso positivo. Aquí hay tres fragmentos de Boletia. En los tres se ve un olor del catálogo. En uno de los tres, el olor es un falso positivo y el código está bien como está. Identifica cuál y explica por qué los otros dos sí son problemas reales.
# (a) utils/currency.py
def format_money(amount, currency):
if currency == "MXN": return f"${amount:,.2f} MXN"
elif currency == "USD": return f"US${amount:,.2f}"
elif currency == "EUR": return f"{amount:,.2f} €"
elif currency == "COP": return f"${amount:,.0f} COP"
elif currency == "ARS": return f"${amount:,.2f} ARS"
else: return f"{amount:,.2f} {currency}"
# (b) payments/stripe_provider.py
class StripeProvider(PaymentProvider):
def __init__(self, client):
self.client = client
def charge(self, order):
r = self.client.create_charge(amount=int(order.total * 100), currency="MXN")
return ChargeResult(ok=r["status"] == "succeeded", reference=r["id"])
def refund(self, order):
return self.client.create_refund(charge_id=order.external_ref)
# (c) reports/attendees.py
def attendee_row(ticket, order, customer, event):
return {
"name": customer.name,
"email": customer.email,
"ticket": ticket.id,
"type": ticket.kind,
"seat": ticket.seat or "-",
"event": event.name,
"paid": order.total,
"status": order.status,
}
Ver solución
El falso positivo es (a).
Sí, es un if/elif de cinco ramas sobre un tipo, y a primera vista es el mismo olor que investigamos en calculate_price. Pero mira las tres diferencias que lo salvan. Primero, no hay comportamiento: cada rama es una plantilla de texto, no una decisión de negocio. Segundo, no se repite en ningún otro lado: la pregunta "cómo se escribe un monto en esta moneda" se contesta en un único lugar del sistema, así que no hay riesgo de desincronización. Y tercero, el else es un default correcto, no un agujero: una moneda desconocida se formatea de forma razonable en vez de tomar una decisión por omisión. Convertir esto en una jerarquía de clases Currency con un método format() sería exactamente lo que la lección llamó corregir de reflejo: agregar cinco clases para eliminar cinco líneas.
(b) sí es un problema, pero uno chiquito y de otro tipo. Lo que salta es que "MXN" está escrito a mano dentro de charge — un valor de negocio enterrado en un detalle de infraestructura. Consecuencia concreta: el día que Boletia venda un evento en otra moneda, este proveedor va a cobrar en pesos sin avisar. Ahora bien, la clase en sí está bien: es un Adapter haciendo su trabajo —traducir entre el SDK de Stripe y la forma común de Boletia—, y el hecho de que casi todo lo que hace sea delegar no es middle man. Ese es el falso positivo que hay que saber descartar: la delegación con traducción tiene valor.
(c) sí es un problema, y es data clump. Los cuatro parámetros —ticket, order, customer, event— viajan juntos, y casi seguro viajan juntos en varios lugares más del módulo de reportes. Consecuencia: cuando haga falta un quinto dato en la fila —el proveedor de pago, por decir algo— hay que cambiar la firma de esta función y de todas las que la llaman. Además, el llamador tiene que conseguir las cuatro cosas antes de poder pedir una fila, lo que suele producir la cadena de consultas que vimos en el ejercicio 2. Lo que aquí falta es un concepto que no tiene nombre: algo como AttendeeRecord, que sepa juntar las cuatro piezas una sola vez.
Por qué funciona: el ejercicio entrena la habilidad que separa el vocabulario útil del vocabulario dañino. Reconocer la forma de un olor es fácil y se aprende en una tarde; decidir si esa forma corresponde a un problema real requiere leer, contar y pensar en el costo. Si en tu próxima revisión descartas conscientemente un olor que viste —y lo dices, aunque sea solo para ti—, ya estás usando el catálogo como se debe.
Resumen y siguiente paso
En esta lección definiste qué es un code smell: un rasgo superficial y observable del código que, con frecuencia alta, indica un problema de estructura más profundo. Viste que las tres palabras cargadas de esa definición descartan tres malentendidos: superficial significa que se detecta sin conocer el dominio y por eso es enseñable; observable significa que se cuenta y por eso sale del terreno del gusto; y con frecuencia alta, no siempre significa que un olor es una correlación y no una implicación —de ahí que exista el falso positivo legítimo—.
Viste la anatomía de tres piezas que vas a usar el resto del módulo: el síntoma (un hecho contable), la hipótesis (una interpretación sobre la estructura) y el costo (una predicción sobre trabajo futuro). Y viste el ciclo completo funcionando sobre pricing/calculator.py, con un desenlace que conviene recordar: la investigación cambió el hallazgo —de "un if largo" a "dos decisiones que se van a tomar por omisión"—, el resultado se partió en dos comentarios de peso distinto, y la mayor parte del código quedó exactamente igual que como entró.
Te llevas el catálogo corto —doce olores agrupados por la pregunta que responden— con su ejemplar en Boletia, y la regla que ordena todo el módulo: un olor detectado no autoriza un cambio, autoriza una pregunta, y esa pregunta tiene tres partes en orden: ¿es real?, ¿cuánto cuesta?, ¿ahora?
Antes de avanzar deberías poder: explicar por qué un olor no es un error; dar un ejemplo de falso positivo y decir qué lo salva; nombrar los cinco grupos del catálogo; y describir la simetría entre divergent change y shotgun surgery.
Lo que sigue es la lupa. Tres de los doce olores concentran la mayor parte del dolor real y aparecen en casi todas las revisiones: God object, feature envy y shotgun surgery. La lección 3 los abre uno por uno con su ejemplo en Boletia, y para cada uno te da algo que esta lección solo insinuó: un método de detección casi mecánico —qué contar, con qué comando, qué número es señal— y la dirección hacia la que suelen resolverse.
Recursos
- Refactoring, capítulo 3 — Bad Smells in Code (Martin Fowler y Kent Beck) — el capítulo fundacional, escrito en 1999 y todavía vigente. Vale la pena por una razón que este resumen no puede transmitir: los autores insisten una y otra vez en que no hay reglas, solo señales.
- Refactoring Guru — Code Smells — el catálogo completo agrupado en cinco familias, y para cada olor las refactorizaciones que suelen aplicarse. Consulta, no lectura corrida.
- Wiki Wiki Web — Code Smell — la discusión original de la comunidad donde nació el término, con Kent Beck explicando de dónde salió la metáfora del olor. Es un documento histórico y se lee en diez minutos.
- Your Code as a Crime Scene (Adam Tornhill) — cómo usar el historial de control de versiones como evidencia sobre el diseño. Es la fuente de la idea de que los archivos que más cambian son un dato sobre la estructura, no solo sobre la actividad.