Code Review Checklist

Durante estos últimos días he estado hablando, con mi amigo Israel Alcazar (@ialcazar), bastante sobre el Bus Factor porque tenemos una charla pendiente que preparar para ALE2013 en Bucarest, y a parte porque a ambos nos gusta charlar de cosas relacionadas con Agile, y solemos tener conversaciones “casuales” para hablar de distintos temas.

A partir de ahí, empezamos a pensar como sería posible aumentar el Bus Factor (número de personas que deben ser atropelladas por un autobús antes de que el proyecto se vea en serio peligro), y una de las posibles ideas que surgió fue la gestión del conocimiento a partir de Pair Programming y Code Review.

Suele suceder que cuando un proyecto empieza el Bus Factor es igual a 1, por lo que rápidamente hay que empezar a gestionar ese conocimiento de una forma eficiente. Casualidades de la vida, el karma o la ley del famoso Murphy, justo cuando estamos hablando de estos temas, surge un proyecto nuevo de cierta importancia para nuestra empresa, y bueno uno de los miembros del equipo empieza a preocuparse por la prevención de bugs en producción y nos preguntó por la técnica de Code Review, ya que había estado leyendo sobre ello y le llamó la atención.
Para los que no halláis oído hablar de esta práctica, el Code Review es una técnica mediante la cual se revisa el código fuente por parte de un compañero, un grupo de personas o una herramienta. Además, nos permite asegurarnos que la calidad de nuestro código es lo suficientemente buena como para pasar el juicio y criterio de estos agentes externos al desarrollo en cuestión. Cuando se hace Code Review hay que tener claro dos puntos importantes:
  1. Debe de hacerse desde un punto de vista objetivo e imparcial
  2. No debe usarse como herramienta de castigo hacia el autor del código fuente
Las dos reglas anteriores las tenemos que tener claras antes de empezar, porque si no no podremos aplicar los criterios con objetividad a la hora de hacer Code Review manual, ya sea individual o en grupo.
A partir de que este compañero se interesó por hacer Code Review sobre código Java, decidí intentar pensar en una lista de características que debía de cumplir el código para que recibiera el visto bueno de las personas que lo revisaban. Estas características las he diferenciado en tres grandes grupos: principios, estructura del código y codificación:
  • Principios o premisas iniciales sobre las que sustentar nuestra revisión de código:
    • DRYDon’t repeat yourself: No tengas código duplicado, potencia la reutilización
    • SRPSingle Responsibility Principle: Los métodos/clases solamente deben hacer/representar una y solamente una cosa
    • KISSKeep it Simple, Stupid!: Código fuente fácilmente entendible
    • Clean Code: Código manejable y mantenible
  • A nivel estructural el código debería de tener las siguientes características:
    • Los métodos deben ser pequeños, normalmente se suele definir un número aproximado de líneas como guía
    • El diagrama de clases debe ser lo más simple posible, pero siempre cumpliendo el principio SRP
    • La estructura debe permitir cambios futuros sin problemas
    • Las clases deben estar agrupadas por funcionalidad en sus respectivos paquetes
    • Hay que evitar el acoplamiento, es decir, los módulos o paquetes pueden ser intercambiados por otras alternativas sin problemas
    • Escalabilidad: El código debe portarse perfectamente en caso de que múltiples usuarios accedan a él
    • Verificar el uso de patrones de diseño
    • Evitar la pérdida de encapsulación
    • Identificar posibles cuellos de botella
    • Puntos de escritura en logs lo más eficientes posible
    • Identificación de posible bloqueos por inherencia de recursos en las aplicaciones multithread
    • Dependencias externas, como conexiones con bases de datos o servidores, correctamente manejadas
    • Identificar posibles memory leaks. Esto será complicado de ver en algunos casos y se aconseja complementar con pruebas de stress
  • Codificación o características para el código
    • Comprobar que no existe código innecesario (death code)
    • Comprobar que no existen bucles extra y/o evitar lo máximo posible los bucles anidados
    • Optimizar las sentencias “if-else“, y si podemos prescindir del uso de la clausura “else” mejor que mejor
    • Comprobar el uso de los métodos estáticos y los bloques de inicialización, rompen con el concepto de orientación a objetos
    • Respetar las convenciones de nombrado, longitud de los métodos/clases, formatos, …
    • Revisar los modificadores de acceso por la seguridad del desarrollo
    • Comprobar si existen tests unitarios que comprueben el código, si es así revisar que estos tengan sentido
    • Identificar posibles excepciones “escondidas
    • Evitar bloques “try-catch” que engloben demasiado código
    • Mensajes de error identificativos/descriptivos
    • Evitar en la medida de lo posible los valores hardcodeados en las variables locales
    • Revisión de posibles RuntimeExceptions, como por ejemplo los NullPointerException
    • Evitar errores típicos como el uso indebido de tipos numéricos como short o int para números muy grandes
Estos son puntos importantes a tener en cuenta cuando se realiza Code Review, pero no son los únicos, y dependiendo de las funcionalidad desarrolladas algunos de ellos aplicarán y otros no, pero aún así siempre es bueno tenerlos presentes. Cada equipo debe de establecer su Code Review Cheklist y adaptarla a sus necesidades. De todas formas, lo más lógico es siempre empezar con un lista de premisas muy relajada e ir revisándola para poder tener más reglas y más estrictas, y así ir aumentando de forma progresiva la calidad del código fuente.

 Aumentar la calidad del código, evitar bugs y esparcir conocimiento entre los miembros de los equipos son los principales beneficios de hacer Code Review, ¿por qué no lo íbamos a poner en práctica?