Revisiones de código. ¿Para qué vale eso?

Seguramente hayáis asistido alguna vez a una revisión de código. No sé si es que me estoy haciendo muy mayor y me vuelvo muy crítico o qué es, el caso es que no le veo mucha utilidad a lo que se hace y se entiende habitualmente por revisión de código.

Resulta que he estado en revisiones de código donde había unos tochos impresionantes de papel (los fuentes de muchos programas), y donde el que lideraba la revisión iba diciendo.

• Página 7 (la que fuere). ¿Alguien tiene algo que decir?
• No.
• Página 8. ¿Algo aquí?
• …

Y de cuando en cuando alguien lanzaba un comentario del tipo

• El código es muy denso y no hay comentarios- o bien,
• Aquí está mal indentado.

Y muy esporádicamente, al llegar a un programa dado, la gente se ponía a discutir sobre los requisitos que dieron lugar a la implementación de los mismos en ese programa. Sobre si los requisitos estaban bien o mal. Si tenían que haber sido de otra forma, pero bueno, no había tiempo para cambiarlos, y cosas por el estilo.

Total, que salía de esas revisiones con un sentimiento extraño de inutilidad. Bien mía; bien de la gente; bien de la reunión en sí.

El tiempo me ha hecho tomar posiciones, que son las que voy a contar aquí y ahora.

Para mí, hablando de revisiones de código, tenemos de 2 tipos:

• Code Peer review (o peer review de código)
• Code Management review (lo mismo)

(Disculpad los nombres en inglés, pero es que creo que están lo suficientemente difundidos como para adoptarlos).

Peer Review de Código

Se trata, como su nombre dice, de revisión entre iguales. ¿Quiénes son esos iguales? Los programadores.

Lejos de chorradas como la anécdota que contaba al principio, las peer review valen para:

• Estandarizar el código
• Refactorizar el código
• Aprender
• Adoptar buenas prácticas
• Resolver problemas
• Tomar decisiones

Vamos con esto. No voy a hablar ahora de cómo se hace una peer review, sino de para qué vale, que es el punto de partida.

Estandarizar el código.

Estandarizar el código sirve como vehículo de comunicación. Es útil para mantener el código. Es útil por si alguien se pira de la empresa y te toca seguir con su trabajo. Es útil porque sirve a los propósitos anteriores y no causa pérdidas de tiempo adicionales.

En una peer review, el comentario del programador senior, o del conductor de la peer review o de cualquiera puede ser:

• ¡Mengano! Estamos todos hasta los piiiiiii de que no uses la notación húngara en tu código como hemos quedado. El próximo programa que vea como este te lo vas a meter por piiiiiii. Y desde luego, coge este código y rehazlo. Coño.

(Lo de “coño” es opcional)

Lo que se haga al respecto de Mengano y su código, pasa al apartado de toma de decisiones.

Refactorizar el código y aprender.

Eso de refactorizar es básicamente reestructurar el código para hacerlo más legible, más óptimo, más eficiente, “más mejor”. Un senior, esto es, un buen programador es capaz de ver un código mediocre y refactorizarlo en su cabeza, y luego, que es la utilidad del tema, mostrarlo a los demás.

En una buena peer review, se levanta el senior, se acerca a la pizarra, y dice:

• A ver chavales, prestadme atención. Fulano, este programa tuyo es un tanto cutre. Mirad como lo doy la vuelta

Y entonces este senior hace una demostración de fuerza y refactoriza el código con la experiencia de sus 10 años, con lo cual, los otros programadores alucinan y aprenden (sí, aprenden de verdad) de una lección magistral que tardarían meses o años en conocer por sus propios medios.

Más tarde, este mismo senior, dice:

• Prestadme atención de nuevo, anda. Mirad este algoritmo de Zutano. Está bien, pero podríamos mejorarlo de esta forma.

Y de nuevo da otra pequeña lección magistral.

Y lo que es mejor, cosas como:

• Atención. Veo que la mitad estáis teniendo problemas para probar el código teniendo en cuenta que no se han programado aún algunas cosas. Os falta un concepto básico. Os voy a enseñar a diseñar y a utilizar un mock.

Hay muchas oportunidades para que un senior dé sus consejos y lecciones a los que no son tan seniors. Eso permite mejorar el código y aprender a gran velocidad. En este sentido las peer review deberían ser como el postre de la comida; lo más deseado por los programadores menos expertos.

Adoptar buenas prácticas y resolver problemas

Se deduce de lo anterior. Problemas en código que por supuesto, compila (no se te ocurra presentarte a una peer review con un código que no vale para nada. No hagas perder el tiempo a la gente) pueden ser resueltos. Los seniors aparte de ayudar a refactorizar, y mejorar el código, pueden presentar buenas prácticas. Patrones y antipatrones.

Los programadores expertos son venerados entre los suyos. Generalizar el uso de patrones, buenas prácticas, etc. es muy fácil cuando te lo dice uno de los tuyos, al que además, respetas, y que adicionalmente te lo pide con amabilidad.

Tomar decisiones

Y finalmente, tomar decisiones sobre el código que se presenta a revisión. Establecer necesidades de refactorización, sugerencias (órdenes en muchos casos) para rehacer alguna parte, adoptar convenios, fijar objetivos, etc.

¿No os parece útil todo esto?

Luego está todo ese rollo de que se emplea mucho tiempo en hacer peer reviews, y que si se quiere revisar un buen tanto por ciento del código es imposible. Aquí mi opinión, como en otras ocasiones, es utilizar la lógica, sobre todo la lógica de los propios programadores (me fío más de la lógica de los seniors, en general). No hay que revisar todo, no. Para empezar hay que olvidarse de revisar todo lo que se pueda automatizar. No revises la indentación, ni si hay comentarios o no por cada montón de líneas de código. Olvídate de la complejidad ciclomática y de cosas similares.

Utiliza una herramienta que permita revisar automáticamente la mayor cantidad de cosas posibles. Toma decisiones, eso sí, en base a los datos aportados por las herramientas.

Revisa todo el código que puedas. El concepto es muy simple. Creo código y pido la opinión de otras personas. No tengo porqué sentarme continuamente alrededor de una mesa con 4 personas para hacer una peer review. De hecho no es muy eficiente. Las peer review pueden ser asíncronas (hago el código esta mañana y esta tarde un par de personas le echarán un ojo). No hacen falta 6 personas en una peer review. Con 2 sería suficiente. Desde luego, lo que no hace falta es ningún perfil de gestión que no sepa de codificar. Sólo mete ruido, hace preguntas poco apropiadas y lo que es peor, espera respuestas que tienen que ver muy poco con una peer review.

Según pasa el tiempo y un equipo coge práctica en hacer revisiones de código, consecuentemente los programadores más junior dejan de serlo, las buenas prácticas se convierten en algo habitual, y son los propios programadores los que refactorizan su propio código antes de ir a una peer review, etc., de modo que la necesidad de hacer peer reviews formales disminuye. El jefe del desarrollo o similar pasa entonces a revisar el código más crítico, a hacer muestreo aleatorio, a revisar más el código de los más juniors, y a mirar los datos que salen de la herramienta de revisión automática. Y los programadores por su parte, siguen pidiendo la opinión de otros en las partes de código donde sienten necesidad de ayuda.

Así de bonita es la vida. Llegar a esto es una cuestión de muchos meses. Conozco unas cuantas empresas dónde esto es lo habitual. En consecuencia, el código que desarrollan es bueno. Conozco muchas más empresas dónde esto no sucede, y el código tiende a ser peor, lo cual se constata fácilmente, entre otras cosas, por el número de incidencias que tienen.

Management Reviews de Código

Estas revisiones van destinadas a los gestores, no a los programadores, aunque pueda participar alguno. Participa el jefe del proyecto, el jefe del desarrollo, etc. Son personas que saben muy bien cómo influyen la decisiones que se toman respecto al código en el desarrollo de los proyectos. Son la gente que asume responsabilidades, que asume las variaciones en la carga de trabajo de la gente, que decide sobre las incidencias del proyecto, que conoce al cliente, que decide sobre los plazos, y sobre todo, que maneja el presupuesto del desarrollo.

Aquí se toman las decisiones que afectan la codificación en general, no al código fuente de un programa concreto.

Las management reviews sirven para:

• Saber cómo se está codificando en general
• Conocer si los objetivos o requisitos que se hubieran establecido para la codificación se están alcanzando
• Modificar los recursos (humanos y técnicos)
• Tomar otras decisiones

Como veis, para mi es algo muy distinto de una peer review. Llevar a cabo management reviews es importante para tomar decisiones que afectan a la globalidad del desarrollo. Por supuesto, se deben tener en cuenta, entre otros datos, los resultados de las peer reviews.

Conocer cómo está el código y el estado de los objetivos

En buena lógica deberían existir unos objetivos establecidos para la codificación. Se debe intentar que los objetivos sean medibles cuantitativamente. La medición debe ser simple, a ser posible, automática. ¿Por qué? Porque es lo que facilita la visualización de los datos y por tanto, tomar decisiones.

No voy a hablar de objetivos aquí. Todo lo que voy a decir es que al final conoces cómo está el código en general, por cosas como:

• Cantidad de código cubierto por pruebas unitarias
• Nivel de acoplamiento y cohesión de las clases
• Complejidad ciclomática
• Etc.

Esto es lo que se conoce como métricas del software. Hay cientos. Hay otros indicadores de cómo está el código, como los que se basan en la opiniones de los seniors expresadas en las peer reviews; métricas relacionadas con las pruebas y defectos, etc. Todo es importante.

Modificar recursos y otras decisiones

Las decisiones de un management review son similares a las de un conductor de un coche que mira la velocidad, las revoluciones, el aceite, la temperatura, las señales de tráfico, la hora, la prisa que tenga, etc. Decisiones basadas en lo interno (el código, los programadores, etc.) y lo externo (compromisos con el cliente, plazos de entrega, etc.). El conductor no es un mecánico. No sabe cómo funciona un cilindro, ni qué es eso de la junta de la trócola, desde luego. Pero lo que sí sabe muy bien es interpretar lo que ve en el salpicadero y acelerar, frenar, cambiar de marcha, coger por esta calle o por esta otra. Eso, los buenos conductores, por supuesto.

Las decisiones de gestión son así. Pongo un programador más en esto; admito una menor cobertura de pruebas en esto otro que no es crítico; exijo una disminución de complejidad ciclomática; pido rehacer toda una funcionalidad; subcontrato un desarrollo en algo que nos supera; etc.

Y nada más. Si tenéis otras opiniones, este es un buen sitio para exponerlas.