Atlasingeniería

Code review en la prácticaLos comentarios que vuelven siempreTema 6

Calcular dos veces lo mismo y otras condiciones difíciles de leer

La misma expresión repetida en tres ramas de un condicional, una condición con cinco cláusulas y un ternario anidado. No es rendimiento: son las tres formas más comunes de esconder un caso que nadie contempló.

Un comentario de review que suena a manía: «esto lo estás calculando dos veces». La respuesta mental es que el navegador lo resuelve en microsegundos y es verdad.

El problema no es el costo. Es que dos cálculos idénticos hoy son dos cálculos que pueden dejar de ser idénticos mañana, cuando alguien corrija uno solo. Y una expresión repetida en tres ramas significa que el if está partido por el lugar equivocado.

El cálculo repetido

Lo que se veLo que suele indicarQué hacer
La misma expresión en dos ramas del condicionalLa condición no separa lo que parece separarSacarla arriba, con nombre
Un cálculo dentro de un ciclo que no depende del cicloSe escribió donde se necesitó, no donde correspondeSubirlo fuera del ciclo
La misma derivación en la vista y en el servicioFalta un dato derivado en el origenCalcularlo una vez, idealmente en el backend
Sacar la expresión y ponerle nombre resuelve el problema de lectura y el de mantenimiento a la vez.

El tercero es el caso grave en un producto con varias plataformas: si el total, el descuento o la elegibilidad se derivan en cada cliente, cada cliente va a dar un número apenas distinto y la diferencia va a aparecer en un reclamo, no en un test.

La condición con cinco cláusulas

Una condición larga es difícil de leer y, sobre todo, difícil de verificar: nadie puede decir de memoria qué casos cubre.

Cómo se desarma

  1. Nombrar las partes. Tres booleanos con nombre —isEligible, hasActiveCard, isWithinWindow— y una condición que se lee como una oración.
  2. Preguntar por el caso que falta. Con las partes nombradas se vuelve evidente qué combinación no está contemplada. Con la condición larga, no.
  3. Salir temprano. Las guardas al principio de la función achatan el anidamiento y dejan el camino feliz sin sangría.
  4. Si son reglas de negocio, que se vean como reglas. Cuando la condición codifica una política —quién accede a qué—, conviene que esté en un solo lugar con nombre, no repartida en tres vistas.

La evaluación en corto que esconde una decisión

La otra forma de esconder un caso es la evaluación en corto: usuario && usuario.perfil && usuario.perfil.nombre. Funciona, y lo que oculta es qué se supone que pase cuando falta cada una de esas piezas.

Antes de seguir, predecí

En una vista aparece `user && user.profile && user.profile.name`. ¿Qué es lo primero que conviene preguntar en la review?

Es el mismo mecanismo que el default silencioso: el código sigue andando y nadie decidió qué debía pasar.

Cuándo la repetición está bien

No toda duplicación local hay que extraerla. Extraer por reflejo produce funciones con nombres genéricos y parámetros booleanos que son peores de leer que las dos líneas repetidas.

La prueba es la misma que con los helpers: ¿si cambia una, tiene que cambiar la otra? Si la respuesta es sí, se extrae. Si las dos expresiones coinciden por casualidad —dos reglas distintas que hoy dan el mismo número—, unificarlas va a obligar a separarlas de nuevo, con más trabajo.

Más a fondo · nivel seniorExtraer con nombre es documentación que no se desactualiza

El beneficio más grande de sacar una expresión a una constante con nombre no es evitar el cálculo: es que el nombre explica la intención en el lugar donde se usa. const isWithinGracePeriod = ... dice por qué existe esa comparación de fechas, cosa que un comentario también haría, con la diferencia de que el nombre no puede quedar desactualizado sin que alguien lo note.

El cálculo que se repite en dos ramas

src/pricing/apply-pricing.ts+9−3

Hay 4 problemas en este cambio. Tocá la línea donde creas que está.

@@ -6,14 +6,24 @@ import { getUserTier } from '../users/tier';
66
77
88
9
10
11
9
10
11
12
13
14
15
16
17
1218
1319
1420

Cuatro comentarios sobre once líneas, y ninguno es sobre rendimiento.

Cuándo repetir está bien

Cierre

Autoevaluación

¿Lo entendiste?

La misma expresión aparece en dos ramas de un condicional. ¿Qué suele indicar?
¿Cuál es la prueba para decidir si dos expresiones repetidas deben unificarse?