La "vieja" práctica de inspección de código ¿sigue manteniendo el tipo?

Todos hemos escuchado/leído las bondades de esta práctica y la mejora substancial en la calidad y mantenibilidad de nuestro código...entonces...¿por qué es tan poco utilizada? Tal vez yo he tenido muy mala suerte... pero pocas veces la he visto claramente integrada en un proceso de desarrollo.

Las inspecciones de código tienen como principal objetivo mejorar la estructura del código fuente, no descubrir bugs (de hecho inspeccionamos los fuentes no los vemos en ejecución) por lo que el hecho de incorporar una fase de revisión en el proceso de desarrollo no implica que puedas evitar la fase de testing. Tal vez este sea uno de los motivos por lo que no es tan utilizada: los tiempos de desarrollo se alargan... no podemos dedicar ese tiempo extra en nuestros proyectos... la calidad es cara y no podemos pormitírnosla (el cliente no la pide, no la paga,... no la tiene), además ya vamos tarde... como para encima dedicarnos a revisar código.... Hasta que alguien plantea una solución intermedia ¿Y que tal si hacemos un manual de estilo y lo sigue todo el mundo? Y ahí acaba todo, en las 100 o 200 hojas de manual de estilo que nadie suele leer.

¿Te suena la escena?



Otro motivo que puede desalentar su uso es lo rancio del nombre, el revisor nunca ha caído bien a nadie... y vaya, comparado con alguno de los nombres tan interesantes que tenemos últimamente... seamos sinceros, Inspección o Revisión Técnica Formal... uff... mal rollo ;-)

Viendo un vídeo de James Whittaker (anteriormente máximo responsable de Testing en Google y por estos días de Microsoft) escuché que en Google todo el código que sube a producción es revisado. Vaya, probablemente los proyectos en Google tampoco irán sobrados de tiempo, pensé yo... y en cuanto al presupuesto en esa misma charla Whittaker argumentaba que el porcentaje de testers que hay en Google frente a otras empresas comparables era sensiblemente inferior, debido a que, en Google el desarrollador también hace mucho Testing... lo cual en principio justifica la inversión (por no hablar del coste de la baja calidad). Total, si ellos pueden... decidimos intentarlo (revisar todo el código que sube a producción) y después de algún que otro ajuste y retraso en su puesta en marcha os cuento la experiencia real.

Establecimos unas reglas básicas:

1. Todo cambio o nuevo desarrollo que implique tocar lógica de negocio será inspeccionado, por otro desarrollador, antes de pasar a la fase de Testing.
2. Cuando finaliza la inspección, el revisor le explica los cambios al desarrollador pero no se discuten las observaciones presentadas. Puede existir debate constructivo pero en ningún caso se discute. La decisión final de si llevar a cabo el cambio propuesto será del desarrollador que ha escrito el código. Él es el autor, suya es la responsabilidad y por tanto la decisión.
3. No se vuelve a revisar el código, una vez el desarrollador realice los cambios oportunos se pasará dicho trabajo a la fase de Testing. No tiene sentido volver a revisar puesto que el desarrollador no está obligado a implementar las "sugerencias" del revisor.
4. La revisión debe ceñirse en la medida de lo posible a los cambios introducidos en el trabajo bajo inspección, mantenemos una gran cantidad de código y no nos podemos plantear una revisión/mejora global.
5. Ante conflictos en los criterios de implementación, primará la claridad del código. El código se escribe una vez pero se lee posiblemente decenas de veces... por lo que si el revisor no entiende fácilmente el código el desarrollador debería  mejorar su legibilidad.

Para arrancar el proceso revisé el código de determinados cambios que subíamos a producción comentándolo con algún miembro del equipo. El hecho de dejar la revisión abierta dificultaba el proceso de revisión y la discusión posterior por lo que decidimos crear una lista de control de revisión. Un buen punto de partida para dicha lista puede ser el Code Complete de McConnell o el Clean Code de R.C. Martin,  pero es altamente recomendable la participación del equipo (van a ser revisores y revisados) para que no existan problemas de consenso en cuanto a los detalles de la revisión.

Un aspecto crucial: no se pretende ganar ningún concurso de programación, la mayoría de los puntos a revisar forman parte de cualquier curso de fundamentos de programación. Cuando programamos nos enfocamos en la resolución de un problema, cuanto más complejo sea el problema más atención nos requerirá y por tanto menos atención prestaremos a determinados detalles de la implementación. De ahí que el revisor,  liberado de la resolución del problema, tenga mayor facilidad para fijarse en esos detalles. El hecho de complicar en exceso la lista de control provocará discusión y enfrentamiento... Ponlo a prueba e  inspecciona  fundamentos simples, probablemente te sorprenderá lo que se llega a mejorar el software que subes a producción.

A continuación te dejo algunos de los aspectos sobre los que se definió la primera aproximación. Es un ejemplo, yo no la utilizaría fuera del grupo que la definió, como dije antes, creo que es preferible que el equipo que desarrolla/revisa defina sus propias reglas, además debe ser adaptada a la tecnología utilizada por el equipo.

Cohesión de un método/clase: se analiza cómo de relacionadas están las operaciones llevadas a cabo dentro de un método/clase.

Modificadores de acceso. Verificar que la visibilidad establecida es la adecuada para optimizar la encapsulación de la clase/componente.

Nombres de los métodos: los métodos deben tener nombres descriptivos que muestren qué operación realizan.

Líneas de código por método: No existe un consenso sobre la idoneidad de un tamaño para los métodos pero definimos las siguientes pautas a seguir:

• Límite inferior: 1 línea de código. Es perfectamente justificable en determinadas circunstancias..
• Límite superior: es una buena práctica que el código se visualice en una pantalla sin utilizar las barras de desplazamiento (facilita su seguimiento) aunque en determinadas circunstancias puede ser perfectamente justificable métodos más grandes.

Organización interna del código: mantener una estructura interna ordenada de modo que se pueda seguir fácilmente el flujo de control. Evitar mantener código comentado que dificulta la lectura del código real. Evitar los comentarios de código obsoletos.

Parámetros:

• Verificar que todos los parámetros de la firma del método son necesarios 
• Verificar que los parámetros se pasan correctamente por valor o por referencia. 

Evitar los “números mágicos”. Éstos son números que aparecen en medio del código sin saber qué representan. Algunos se describen solos (¿Intuyes qué representa este? 3,1416) pero la mayor parte de estos números no son tan intuitivos y de hecho suelen cambiar con el tiempo.

No incluir código “hardcoded”: Incluir los literales que se utilicen para el control del flujo en ficheros de configuración de la aplicación o a una tabla. Ni qué decir con las consultas SQL...

Creación de controles de usuario Cuando un tabla se repite (o la mayor parte de la funcionalidad) en dos o más páginas, debe encapsularse esta funcionalidad en un control de usuario, para evitar duplicidades de código y su posterior mantenimiento.

Evitar código duplicado: Bestia negra de la mantenibilidad... cuando tenemos clones en el código, un cambio implica tener que modificar todos los métodos duplicados.

Tipos de datos ajustados al valor almacenado en la variable: Ajustar el tipo de datos a la variable declarada para optimizar el proceso y el consumo de memoria. Especial atención a estructuras de datos grandes.

Evitar código muerto: Eliminar el código inalcanzable o muerto que no se va a usar. Situación típica: se va a modificar un método pero como no estamos seguros del cambio se duplica con otro nombre similar, cuando se termina se sustituye por el anterior...pero los dos métodos se mantienen ahí.

No aumentar el número de alertas de compilación respecto a los ya existentes. Contribuir al desarrollo correcto del código y al menos no dejarlo peor de lo que estaba.

Verificar las métricas: la complejidad ciclomática, acoplamiento de clases y la profundidad de herencia se encuentra en los parámetros predefinidos.

Excepto tal vez esta última última, como decía, todas son de fundamentos de la programación.

Obviamente a ninguno se nos escapa que cuando tiramos de una de las restricciones del proyecto (calidad en este caso) alguna otra se verá resentida...¿tienpo? ¿costo? Bueno esto en otro momento, pero mis datos indican que aunque a corto podamos sufrir un deterioro en otra restricción... a medio sí es mantenible.