Atlasingeniería

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

Valores sueltos que deberían ser constantes o enums

El comentario más frecuente de todos. No es una cuestión de prolijidad: un texto suelto repetido en tres archivos es una comparación que algún día va a fallar en silencio, y el compilador no puede ayudar.

Si tuviera que apostar a qué comentario aparece más veces en un año de revisiones, sería éste: «esto debería ser una constante». Es tan común que se lee como manía de estilo, y no lo es.

El problema del valor suelto no es que sea feo. Es que la misma cadena escrita en tres lugares son tres oportunidades de escribirla distinto, y cuando eso pasa nada falla: simplemente una comparación deja de dar verdadero y una funcionalidad desaparece sin ruido.

Cómo se rompe, en concreto

El valor sueltoQué pasóCómo se detectó
Un identificador de estado escrito a mano en la vista y en el servicioUno quedó con mayúscula distinta al renombrarUn usuario reportó que no veía el cartel
Un número de días de vigencia repetido en tres módulosSe cambió en dosDiferencia entre lo que mostraba la pantalla y lo que aplicaba el backend
El nombre de una clave de almacenamiento localSe escribió con guión en un lado y con guión bajo en otroLa preferencia no se recordaba en una de las pantallas
Un código de país en un condicionalSe agregó un país nuevo y nadie encontró todos los lugaresSe descubrió meses después, con el formato de fecha equivocado
Ninguno tiró un error. Los cuatro se descubrieron por el comportamiento.

El patrón es siempre el mismo: el valor suelto convierte un error de compilación en un error de comportamiento. Con un enum, escribir mal el nombre no compila; con una cadena, compila y no hace nada.

src/orders/order-badge.tsx+4−0

Hay un problema en este cambio. Tocá la línea donde creas que está.

@@ -8,6 +8,10 @@ export const OrderBadge = ({ order }: Props) => {
88
99
10
11
12
13
1014
1115

Un caso real, recortado: el comentario de la review fue de una línea y evitó un bug que no tira ningún error.

Cuándo sí y cuándo no

No todo literal tiene que ser una constante. Extraer por reflejo produce archivos llenos de nombres que se usan una sola vez y no aclaran nada.

La prueba de los tres criterios

  1. ¿Se repite? El mismo valor en más de un lugar, aunque sean dos. Ahí la constante no es opcional.
  2. ¿Es un concepto del dominio? Un estado, un tipo de comprobante, un canal, una marca. Aunque aparezca una sola vez hoy, pertenece a un conjunto cerrado y va a tener hermanos.
  3. ¿El número dice algo por sí solo? limit = 100 en una consulta paginada se entiende; if (diff > 86400) no, y ahí el nombre es lo que explica. Un 0, un 1 o un '' triviales no necesitan nombre.

Dónde vive la constante

El segundo comentario que acompaña siempre al primero es dónde ponerla, y ahí hay una respuesta que depende del alcance.

Tres niveles

  1. Local al archivo, si sólo la usa ese módulo. Arriba del todo, no en el medio de una función.
  2. En el módulo de dominio, si la comparten varias partes de la misma funcionalidad.
  3. En el paquete compartido, si la usan el cliente y el servidor. Y ahí la regla dura: se declara una sola vez y se genera o se importa. Dos listas de estados escritas en paralelo se separan en el primer cambio.

Antes de seguir, predecí

El backend define los estados posibles de un pedido. El front necesita compararlos. ¿Qué conviene?

El nombre también se revisa

Una constante mal nombrada es apenas mejor que el literal. Los comentarios habituales sobre esto apuntan a tres cosas: que el nombre diga el concepto y no el valor, que incluya la unidad cuando la tenga, y que no arrastre el contexto de dónde se usó primero.

THIRTY_DAYS envejece mal el día que pasa a sesenta. DEFAULT_SEARCH_WINDOW_DAYS sobrevive al cambio, dice la unidad y se entiende sin ir a buscar de dónde salió.

Más a fondo · nivel seniorCuando el valor es de negocio y cambia

Hay valores que parecen constantes y son configuración: un umbral de promoción, un porcentaje, una fecha de corte. Si alguien de negocio puede querer cambiarlos sin un despliegue, el lugar correcto no es el código sino una tabla o una bandera. La señal de alarma es un ticket que dice «cambiar el valor de X»: si aparece dos veces, ese valor no era una constante.

Cuándo una constante y cuándo no

Cierre

Autoevaluación

¿Lo entendiste?

¿Cuál es el riesgo concreto de comparar contra una cadena escrita a mano en varios archivos?
Un ticket pide cambiar el valor de un umbral por segunda vez en el año. ¿Qué indica?