Métricas para mejorar nuestro código

Unas de las tareas divertidas que me toca hacer durante el día es el Code Review de varios proyectos. Es una práctica que utilizamos a diario en Patagonian Tech porque nos permite ayudar a los más nuevos a crecer como programadores y a su vez a mantener los productos que desarrollamos para nuestros clientes con un nivel bajo de errores.

Humor: posible métrica para evaluar el buen código.

El otro día estaba haciendo un review a uno de mis compañeros, el código no tenía falencias en su lógica, tampoco en su sintaxis. Luego de mirarlo una y otra vez me puse a pensar que “sonaba complicado”.

El fragmento de código en cuestión es el siguiente :

https://gist.github.com/Gazer/e12f6935bbdf405a97d5b045ff510544

En la primera pasada no acote nada, pero luego de estar dando vueltas un rato por mi cabeza, algo comenzó a hacer ruido.

Usualmente me gustan los códigos cortos, de pocas líneas, porque cada línea de código que agregamos es un potencial punto para introducir un bug. Pero no un bug hoy, uno a futuro, cuando otro programador pase y quiera tocar algo.

En el ejemplo de arriba, ¿qué pasa si hay otro Exercise que tiene una unidad diferente de “minutos”? ¿Debo agregar otro else if?. ¿Qué pasa si esa misma lógica está en otro lado del sistema? ¿Tengo que buscar por todo el proyecto?; ¿cómo sabe el programador que debe hacer eso?; ¿qué pasa si escribo las constantes “(floor_count)” mal?.

Como podemos ver, son muchos “qué pasa si”. Y cada uno es un potencial bug en la vida del proyecto.

Pero antes de refactorizar, también deberíamos preguntarnos : ¿vale la pena refactorizar?, usualmente un refactor implica un cambio; un cambio implica que podemos romper algo; lo que implica que debemos probar todo a fondo; lo que se traduce en tiempo y el tiempo es dinero.


Afortunadamente podemos sacar varias métricas de nuestro código que nos permiten elegir qué código necesita más urgente un refactor que otro. Acá un resumen de las que me parecen que pueden ayudarnos.


Número de líneas

Yo personalmente vengo del mundo de Ruby, donde un método siempre es bueno que sea corto, si es de una línea mejor, ya que es más simple con solo mirarlo que problemas podemos tener.

La “regla del pulgar” para mi suele ser que lo más corto que cumpla su función. Pero si consideramos el código de arriba como la mínima expresión, nada debería cambiar, verdad?

La realidad es que para apoyar esta teoría de contar la cantidad de líneas debemos apoyarnos en otras teorías, como ‘Principio de responsabilidad única’ y la ‘Ley de Demeter’.

El primero dice que una clase debe ser responsable de hacer una sola cosa. La segunda habla de que debemos tener bajo acoplamiento entre nuestros componentes.

Complejidad Ciclomática

Esta métrica nos permite medir “que tan complejo” es nuestro programa, teniendo en cuenta las ramificaciones, bucles, etc. Obviamente cuanto menor es la complejidad, mejor nos va.

Explicándolo de forma simple, la complejidad se calcula a partir de crear un grafo con la estructura del código y luego se puede calcular con una simple fórmula en base a la cantidad de nodos y aristas. Tambien hay herramientas que permiten calcular automáticamente la complejidad.

En este caso concreto la complejidad es baja (da menor de 10) por lo que no sería la principal métrica para decidir que se merece un refactor.

Número de entidades

Esta métrica a mi me gusta para saber un poco el acople del método actual con el sistema en general. Básicamente lo que hago es contar cuántas clases intervienen en la solución del problema. En el caso de nuestro análisis tenemos 2 : Exercise y ExerciseEntry. ¿Es necesario tratar con las dos directamente?

Basta con saber que ExerciseEntry está compuesto por un Exercise para que la respuesta sea clara y apoyándonos de nuevo en la “Ley de Demeter” seguramente la respuesta será que no.


Refactorizando el problema

Si miramos bien el código, el programa anterior se trata de transformar una lista en otra cosa. En esta transformación lo primero que se necesita es saber la unidad de medida. ¿Es responsabilidad de este “tranformador” calcular la unidad de la entrada de un ejercicio?; ¿O cada ejercicio debería saber que unidad tiene?

La respuesta correcta es la segunda, ya que aplicamos el “Principio de responsabilidad única“. Lo más razonable es pasar la lógica para calcular la unidad a la clase Exercise. El primer refactor nos deja el código como sigue :

https://gist.github.com/Gazer/b4f126a5a193577af089ca04fe1035e5

El bloque que realiza la conversión se ve mucho más limpio ahora. Y es más fácil de revisar a simple vista si hay algún error. Además podemos probar con mayor claridad que la detección de la unidad de un ejercicio sea correcta y hasta podríamos agregar pruebas automatizados para prevenir errores futuros. También si analizamos la complejidad ciclomática de cada método por separado encontraríamos una disminución de esta métrica, siendo algo positivo.

La “Ley de Demeter” por su lado nos dice que debemos tener ‘bajo acople’ entre los componentes. ¿Qué significa esto específicamente?. Supongamos que por algún refactor definimos que cada ExerciseEntry va a tener su propia unidad independientemente de lo que diga el Exercise asociado. El refactor va a ser complejo ya que debemos buscar por todo el código las llamadas anidadas y seguramente en algún lado va a quedar un exerciseEntry.exercise.unit() que en algún momento quedará como un bug.

Para lograr bajo acople lo que debemos hacer es nunca saltear al intermediario, aun si eso hace que tengamos un método como “proxy”. En este caso la idea seria llamar simplemente a exerciseEntry.unit() quedando el código de la siguiente manera :

https://gist.github.com/Gazer/b73eaed7a0fd98091651e393feaf1faa

Con un poco de magia de Groovy podemos dejar las cosas un poquito más organizadas :

https://gist.github.com/Gazer/9377ae32c97b4f644e79c04febcd4909

Ahora es la parte donde van al principio del artículo y comparar cual de los dos es más claro, y cual creen que podría tener menos errores.

El fragmento de código original de 17 líneas fue reducido a 7. El objetivo de este fragmento es muy claro ahora y ganamos dos métodos auxiliares que seguramente serán útiles en el futuro, ya que esas clases se utilizan en otros lados del sistema.


Cerrando

Podemos concluir que no solo debemos escribir el código que cumpla con lo pedido, también tenemos que escribir código que, sin llegar a ser críptico e inentendible, sea conciso y robusto, que resiste el tiempo y que minimice la posibilidad de que un tercero introduzca un bug.

Es importante configurar herramientas como GMetric en los sistemas de integración continua para atacar estos problemas antes de que pasen al olvido. Adicionalmente también continuar instruyendo y debatiendo en los code reviews para seguir aumentando la calidad de nuestro software.

“Funciona” no es suficiente.


No es raro en esta profesión, al revisar el trabajo de otro encontrarse con algo que no es del todo semánticamente correcto o que puede escribirse de mejor manera. Tampoco es raro que nos digan “pero así funciona”. Funciona no es, o no debería al menos, ser suficiente.

En el desarrollo de software existen muchos otros factures a considerar a futuro que no siempre tienen que ver con el funcionamiento actual de la aplicación : ¿el código se entiende?, ¿es eficiente?, ¿es fácil de extender si debemos agregar funcionalidades?. Quizás el aspecto más importante desde el punto comercial es que tanto cuesta corregir un error detectado, ya que en códigos escritos de manera pobre, corregir un error y la posibilidad que aparezca uno nuevo suele ser mayor en los casos donde no se tiene una buena disciplina.

En las siguientes secciones trataré de comentar lo que me ha servido a lo largo de los años y qué herramientas podemos utilizar. Algunos ejemplos serán específicos de Android, pero se pueden extrapolar a cualquier tecnología.


Métricas, métricas y más métricas

Un aspecto importante para saber dónde estamos parados y hacia dónde vamos es la utilización de métricas en nuestro código. Estas métricas pueden ser muy variadas, desde contar a mano cuántas líneas de código hay por archivo, clase o método, hasta medir la complejidad ciclomática de nuestro código.

Las métricas deben ser una herramienta que nos orienten, no que guíen nuestro desarrollo. Es decir, no deberían condicionar si o si la forma de escribir una porción de código porque la métrica da peor en ese caso.

Esto hace que las métricas en si mismas no sean lo importante. Lo que nos debería importar es la tendencia de las mismas. Para poder medir tendencia entonces debemos poder consultar un histórico, por lo que con cierta regularidad deberíamos calcular esas métricas, anotarlas y ponerlas en un gráfico.

Siempre que la tendencia sea negativa es un buen signo, que se mantenga no está mal. Si la curva crece con pendiente leve en un período corto y luego se estabiliza o baja no debería ser preocupante.

Nuestros puntos de alarma deberán ser crecidas de más de un 30/40% de golpe entre una medición y otra. En general cuando hay picos de crecimiento es porque se está trabajando en alguna característica nueva y todavía no se empezó la etapa de estabilización. Lo importante es atacar el problema en ese momento y ver donde podemos mejorar.

Automatización : la clave para no abandonar

En la teoría esto de las métricas es fantástico. Hasta que llega nuestro Manager o Jefe y nos dice “pero teníamos que salir ayer a producción” y todo se va al diablo :). Escribimos lo que podemos para que “simplemente funcione”, no tomamos métricas y a veces ni probamos :).

Luego de dos o tres veces que salimos del apuro nuestro código seguramente ya sea inmanejable y si calculábamos las métricas a mano perdimos historial importante para ver la evolución.

La automatización de esta tarea es crucial, ya que nos permite ir calculando de manera automática y de manera periódica la salud del proyecto.

Para automatizar es que existen los servidores de “Integración Continua” (o CI de sus siglas en inglés) como Jenkins o Travis. Estos son lo suficiente flexibles para ser configurados según nuestras necesidades y contienen cientos de plugins que nos harán la vida más fácil.

En el caso de Jenkins podemos usar los plugins de “Android Lint” y “Report violations” para recolectar métricas y que nos haga un gráfico de la evolución de las mismas.

Personalmente no me gusta que el proyecto se marque como “fallado” si las métricas no están en cero. Prefiero simplemente un aviso de que la aplicación está “inestable” y definir yo si tengo un problema o no.

Métricas en Jenkins

Como punto final, queda definir en cada caso cada cuánto corres las métricas. Podría ser por hora, a una hora fija, por semana, etc. Dependerá del proyecto.

Yo prefiero calcularla en “tiempo real” cada vez que alguien sube código al proyecto. Para eso solo debemos configurar el servidor de CI para que detecte que hay un nuevo código, lo baje y corra las métricas.

Las Herramientas

Para ir terminando por hoy vamos a mencionar algunas de las herramientas que podemos usar para medir métricas :

  • FindBugs : Java/Android. Detecta errores comunes y recomienda buenas prácticas
  • PMD : Java/Android/Objective-C. Analiza el código en busca de variables sin uso, condiciones que se pueden simplicficar, objetos que estamos creando de más, etc.
  • Lint : Existe un programa “lint” casi por cada lenguaje/framework que existe. Existen desde 1979, usalo :)!
  • Codenarc : Groovy/Grails. Permite analizar malas práticas, errores de estilo, etc.
  • Checkstyle : Java/Android. Chequeos básicos para escribir Java con buenas prácticas (nombres de variables, organización del código, etc).
  • Clang Static Analyzer : Objective-C/iOS. Similar a Checkstyle.

Siempre se puede mejorar …

Para cerrar, simplemente eso. Siempre se puede mejorar. Lleva tiempo? Al principio un poco, si. Pero una vez que el camino está sentado el proyecto avanza solo.

A medida que veamos los warnings de las herramientas iremos recordándolas, entonces al escribir el código antes de todo ya lo haremos pensando en buenas prácticas, minimizando el problema desde el principio.

Por último y no por eso menos importante, nuevamente decirles que estas herramientas deben ser una guía!, no deberíamos buscar el Santo Grial de tener en cero nuestras métricas, eso podría llevarnos a lugares peores que no tener métricas. Vean tendencias y saquen conclusiones 🙂