Hablé con un amigo y me dijo una frase que alertó a mi sistema inmune: “a mi líder técnico no le gustan los comentarios en el código. Dice que este tiene que ser autoexplicativo”. Es una frase muy común, pero que confunde un par de conceptos muy importantes de diferenciar. Me explico.
Existen tres capas de entendimiento respecto a un feature:
- El intuitivo / usuario. Es el que un usuario de dicho feature te puede explicar. Quiero lograr X, hago Y. No uso User Stories en mi día a día, pero imagino que se puede asimilar al título de una US.
- La lógica de negocio. Es el que te explica un product manager y que mapea todos los Y que logran X, en qué casos son válidos y cómo deben cumplirse ciertos requerimientos no funcionales como integridad, privacidad, rapidez, etc.
- El código. Es la implementación de lo solicitado en la capa 2. Este puede cumplir lo de la capa 2 de muchas maneras distintas. Algunas buenas, algunas malas.
Cuando una persona dice que el código debe ser autoexplicativo, busca comunicar que la capa 3 debe ser un reflejo transparente de la capa 2. El código no debe agregarle complejidad o interrogantes a lo dicho en la capa 2. Estoy de acuerdo.
Miremos un ejemplo donde un comentario intenta “parchar” complejidad artificial e innecesaria en un método que busca y modifica documentos antes de retornarlos:
const getDocumentsForProvider = (
idProvider: number
): Promise<Record<number, Document>> => {
const [documents, clients] = await Promise.all([
this.services.getDocumentsByProviderId(idProvider),
this.services.getClients(),
])
// Estructuramos el objeto que necesita el frontend.
// Requiere el nombre del cliente.
// Nos interesa pasarlo como objeto para que el frontend
// pueda estructurarlo como árbol más rápidamente.
return Object.fromEntries(
documents
.filter(doc => doc.valid)
.map(({ id, fiscalUUID, date, idClient }) => [
id,
{
documentID: id,
idLegalEntity: fiscalUUID,
emissionDate: date,
origin: "documents-api",
clientName:
// Manejamos el caso donde no tenemos
// información del cliente
clients.find(c => c.id === idClient)?.name ?? "Unknown Client",
},
])
)
}El código de arriba tiene unos detalles que al menos a mí me perturban: se retorna un objeto complejo y poco intuitivo; se filtran documentos, luego se quitan atributos y se agregan otros (donde algunos pueden o no estar). Los comentarios intentan explicar por qué se usa un objeto en vez de un arreglo, por ejemplo. Eso es complejidad de capa 3, no de lógica de negocio. El código, efectivamente, debería ser autoexplicativo. No deberían levantarse preguntas de qué es lo que se está retornando.
Pero miremos otro ejemplo de un método que maneja lógica contable de un movimiento entre cuentas.
const persistAccountingMovements = (
idAccountOrigin: number,
idAccountDestination: number,
amount: number
): Promise<void> => {
const [accountOrigin, accountDestination]: [Account, Account] =
await Promise.all([
this.getAccountById(idAccountOrigin),
this.getAccountById(idAccountDestination),
])
if (!accountOrigin || !accountDestination) {
throw new ResourceNotFoundError("Alguna de las cuentas no se encontró")
}
const originIncreases = amount > 0
let accountDebito: Account
// Una cuenta de activo o patrimonio positivo (Ingreso,
// capital, utilidades acumuladas) al aumentar
// significan un débito contable.
// Una cuenta de pasivo o de patrimonio negativo
// (costo, gasto) al aumentar significan un crédito contable.
// En una operación contable siempre hay una cuenta débito
// y otra crédito, para cumplir la ecuación fundamental
// Activo + Pasivo = Patrimonio.
// Para la cuenta de origen la decisión se puede ver como tabla:
// || ============================================================= ||
// || originIncreases \ increaseIsDebito | true | false ||
// || true | Debito | Crédito ||
// || false | Crédito | Debito ||
// || ==============================================================||
// @see https://www.patriotsoftware.com/blog/accounting/debits-and-credits/
switch (true) {
case originIncreases && accountOrigin.increaseIsDebito:
accountDebito = accountOrigin
case originIncreases && !accountOrigin.increaseIsDebito:
accountDebito = accountDestination
case !originIncreases && accountOrigin.increaseIsDebito:
accountDebito = accountDestination
case !originIncreases && !accountOrigin.increaseIsDebito:
accountDebito = accountOrigin
}
const accountCredito =
accountDebito === acccountOrigin ? accountDestination : acccountOrigin
return this.services.saveAccountingMovement({
accountDebito,
accountCredito,
amount: Math.abs(amount),
date: new Date(),
})
}En este caso el comentario es muy distinto: Intenta cubrir el (¿la?) gap entre la capa 1 y la 2: entre una usuario que hace un movimiento de dinero y el PM que explicita la lógica de negocio contable para este caso hay un gran espacio. Un programador no necesariamente sabe de contabilidad para encontrar evidente este comportamiento. Y ninguna cantidad de refactorización podrá suplir esa necesidad de información de contabilidad.
En resumen: un comentario que intenta tender puente en la brecha entre las capas 2 y 3 es nocivo y un code smell. Un comentario que tiende puente entre las capas 1 y 2 es una ayuda para entender las precondiciones y requerimientos del software en un punto preciso del tiempo. No ver esto es peligroso y se termina abusando de la memoria frágil y la errática comunicación oral. Eso, o se tienen páginas y páginas de Notion de documentación.