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.
- Debe de hacerse desde un punto de vista objetivo e imparcial
- No debe usarse como herramienta de castigo hacia el autor del código fuente
- Principios o premisas iniciales sobre las que sustentar nuestra revisión de código:
- DRY – Don’t repeat yourself: No tengas código duplicado, potencia la reutilización
- SRP – Single Responsibility Principle: Los métodos/clases solamente deben hacer/representar una y solamente una cosa
- KISS – Keep 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
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?