Cuando se introducen las revisiones de código dentro del ciclo de desarrollo de un equipo, suele ser habitual que surjan ciertas inquietudes, suspicacias e incluso recelos. ¿Por qué hacer revisiones de código? ¿Quién las va a hacer? ¿Qué se va a revisar? ¿Son realmente necesarias?
Más allá de esto, leer el código escrito por otros desarrolladores constituye una de las mejores prácticas para mejorar como programador y para escribir buen código. Tanto es así, que igual se aprende del buen código como del malo reconociendo patrones y prácticas a evitar. Hoy, Jordi Egea, que ocupa el puesto de Head of development en SlashMobility, se encarga de aclararnos todo esto.
¿qué es un code review?
Es necesario partir de la base de que el equipo trabaja mediante metodologías ágiles, en concreto con Scrum. En Scrum, la especificación de requisitos viene dada por la historia de usuario y la iteración de tiempo en el que se desarrolla dicha historia de usuario es el sprint. Una revisión de código o Code Review, es el proceso mediante el cual los distintos miembros de un equipo revisan en conjunto la implementación realizada por otro miembro para una historia de usuario, aportando ideas sobre mejoras en la implementación, posibles refactorizaciones, discusión de posibles bugs, optimizaciones, errores o mejoras de arquitectura, conveniencia o falta de cobertura de test, y todo tipo de discusiones sobre los cambios realizados para entregar una determinada funcionalidad. Los beneficios de estas revisiones de código van, desde mejorar la calidad del código escrito y el crecimiento profesional de los desarrolladores, hasta ver cómo otros comapeños solucionan problemas concretos e incluso el compartir conocimientos. El objetivo final de una revisión de código debe ser el de determinar qué se está haciendo bien para seguir haciéndolo y qué se puede mejorar. En ningún caso se trata de encontrar culpables ni de fiscalizar el trabajo de los demás. Como resultados más palpables las revisiones de código consiguen:- Mejorar la calidad del equipo.
- Mejorar la calidad de comunicación entre los miembros del equipo.
- Mejorar la calidad del código.
- Como último objetivo es encontrar problemas de codificación.
- Entender el código para aprender por qué y cómo fue desarrollado.
- Tratar de descubrir aquellos patrones y antipatrones para mejorar el desarrollo. No se tratar de revisar clase por clase en busca de posibles errores, se deben buscar patrones.
- Determinar qué se hace bien y qué se puede mejorar, en ningún caso culpabilizar ni señalar a nadie.
- Tratar de cuantificar la calidad del código.
quién ha hecho esto
Inevitablemente, en algún momento se hace bastante habitual preguntarse entre los desarrolladores sobre quién ha hecho el código que tienen entre manos, y es que, el código en el que se trabaja, a buen seguro seguirá siendo mantenido, desarrollado y mejorado en el futuro, cosa que se olvida fácilmente. Por tanto, ¿tiene sentido hablar de la propiedad del código? ¿de quién es el código? ¿del desarrollador? ¿del equipo? Con estas premisas parece que no tenga mucho sentido hablar de la propiedad individual del código y sí hablar de la propiedad y responsabilidad colectiva del mismo. Esto fomenta que un equipo agile, inmerso en al dinámica de estas metodologías, asuma de manera natural el funcionamiento de los entregables. Un equipo de estas características no busca culpables, no apunta con el dedo cuando algo no funciona. Las revisiones de código son una de las formas más efectivas de alcanzar ese ideal.developer guilt
Si eres un desarrollador, probablemente has subido alguna vez a producción un código que contenía algún bug y ha comenzado a fallar. Generalmente, el sentimiento de culpabilidad cuando esto sucede suele ser bastante elevado, al fin y al cabo los desarrolladores son seres humanos (aunque alguna gente no lo crea). Sin embargo, y esto es un pequeño secreto, si se hacen revisiones de código como parte de del proceso de desarrollo, ese error no es culpa sólo del desarrollador. Al menos, no sólo exclusivamente. Existe una fórmula para calcular la culpabilidad del desarrollador: g = 1 / n + 1 Si n desarrolladores han revisado el código, entonces el desarrollador sólo siente 1 / n + 1 culpabilidad cuando envíe un bug. Es una pequeña broma, pero en la práctica, si n desarrolladores han revisado el código, entonces hay n personas que pueden encontrar y corregir el error que un developer ha cometido y no ha visto. git-guilt calcula el cambio de culpa entre dos commits o la culpa de todo el árbol del repositorio en un commit concreto.qué mirar en una revisión de código
Arquitectura / Diseño- Principio de única responsabilidad: La idea es que una clase debe tener una sola responsabilidad. Esto es más difícil de lo que puede parecer. Por lo general, se debería aplicar esto a los métodos también. Si tenemos que usar «y» para terminar de describir lo que un método es capaz de hacer, seguramente podría estar en el nivel equivocado de abstracción.
- Principio Abierto/Cerrado: Si el lenguaje está orientado a objetos, ¿están los objetos abiertos para la extensión pero cerrados para su modificación? ¿Qué sucede si necesitamos agregar otro componente/método?
- Duplicación de código: La regla de los 3 strikes. Si el código se copia una vez, normalmente está bien aunque no sea excelente. Si se copia de nuevo, debe ser refactorizado para que se aísle o separe la funcionalidad común.
- Ofensas visuales: A simple vista, entornando los ojos, ¿es posible identificar formas y patrones conocidos en el código? ¿hay patrones que podrían indicar otros problemas en la estructura del código?
- Refactorización continua: Si se está cambiando un trozo del código que está desordenado es tentador agregar unas pocas líneas y salir. La recomendación es ir un paso más allá y dejar siempre el código un poco mejor de lo que se encontró.
- Errores potenciales: ¿Hay errores o casos que el desarrollador no tuvo en cuenta? ¿Los bucles terminan de la manera esperada?
- Manejo de errores: ¿Los errores se manejan correctamente y de forma explícita cuando es necesario? ¿Se han añadido errores personalizados? Si es así, ¿son útiles?
- Eficiencia: Si hay un algoritmo en el código, ¿está utilizando una implementación eficiente? Por ejemplo, iterar sobre una lista de claves en un diccionario es una manera ineficiente de localizar un valor deseado.
- Nombres de los métodos: Poner nombre a las cosas es uno de los problemas más difíciles en informática. Si un método se llama get_message_queue_name y en realidad está haciendo algo completamente diferente como sanitizar HTML desde la entrada, entonces ese es un nombre de método incorrecto, y probablemente una función engañosa.
- Nombre de las variables: foo o bar probablemente no son nombres útiles para nombrar estructuras de datos, lo mismo que nombrar una variable como e si se compara con excepción. En esto, es necesario ser lo más detallado que se necesite (dependiendo del idioma). Los nombres expresivos de las variables facilitan la comprensión del código cuando hay que volver a revisarlo más tarde.
- Longitud de las funciones: Una regla general es la de que una función debe tener menos de 20 líneas. Si se encuentra un método por encima de 50, probablemente es mejor que se corte en funciones más pequeñas.
- Longitud de las clases: Las clases deben ser de alrededor de 300 líneas en total e, idealmente, menos de 100. Es probable que las clases grandes se pueden dividir en objetos separados, lo que hace más fácil entender lo que se supone que debe hacer la clase.
- Longitud del archivo: Para los archivos Python, alrededor de 1000 líneas de código es lo más que debería tener en un archivo. Cualquier cosa por encima de eso es una buena señal de que debe dividirse en archivos más pequeños. A medida que aumenta el tamaño de un archivo, la capacidad de detección disminuye.
- Docstrings: Para los métodos complejos o aquellos con listas de argumentos más largos, ¿hay un docstring o comentarios que explique lo que hace cada uno de los argumentos si no es obvio?
- Código comentado: Es una buena idea eliminar cualquier línea comentada.
- Número de argumentos en los métodos: ¿Para los métodos y funciones, tienen 3 o menos argumentos? Más de 3 es probablemente una señal de que podría agruparse de una manera diferente.
- Legibilidad: ¿El es código fácil de entender? ¿Es necesario hacer una pausa con frecuencia durante la revisión para descifrarlo?
- Cobertura de tests: Toda nueva funcionalidad debería llevar su tests: ¿Son lo suficientemente rigurosos? ¿Cubren también las condiciones de fallo? ¿Son fáciles de leer? ¿Cómo de frágiles son? ¿Son muy extensos? ¿Son lentos?
- Testing en la medida adecuada: Al revisar los tests, también hay que asegurarse de que se están realizando en la medida adecuada. En otras palabras, ¿cuánto es necesario testear para asegurar que algo funciona adecuadamente? Se recomienda una proporción de 95% pruebas unitarias, y 5% pruebas de integración.
- Número de Mocks: El Mocking es genial, pero mockearlo en exceso no es tan genial. Un regla de oro es que si hay más de 3 mocks en un test, éste debe ser revisado. Puede ser que el test esté probando algo demasiado grande o que la función es demasiado grande. Tal vez no sea necesario ser probado mediante test unitario y que fuera suficiente con un test de integración. De cualquier manera, es algo para discutir.
- Cumplir los requisitos: Por lo general, como parte del final de una revisión, es necesario echar un vistazo a los requisitos de la historia de usuario, tarea o bug. Si no cumple con uno de los criterios, es mejor devolverlo antes de pasarlo a QA.
revisar nuestro propio código primero
Una buena práctica antes de subir nuestro propio código es hacer un git add seguido de un git diff –staged para examinar los cambios que todavía no se han commiteado y buscar cosas como:- ¿Hemos dejado un comentario o un TODO?
- ¿Tiene sentido ese nombre de variable?
- Cualquier otra cosa
cómo manejar las revisiones de código
Debido a que las revisiones de código son fundamentalmente un proceso manual y humano ofrecen un verdadero desafío para manejar ciertas situaciones entre compañeros. Estos son algunos enfoques válidos:- Hacerse preguntas: ¿Cómo funciona este método? Si este requisito cambia, ¿qué otra cosa tendría que cambiar? ¿Cómo podríamos hacer esto más fácil de mantener?
- Reforzar las buenas prácticas: Una de las partes más importantes de la revisión del código es el crecimiento y el esfuerzo de los desarrolladores. Pocas cosas sientan mejor que recibir los elogios de un compañero ante un trabajo bien hecho.
- Discutir en persona los detalles: En ocasiones, es necesario realizar un cambio en la arquitectura, y este puede ser lo suficientemente grande como para que sea más fácil discutirlo en persona y no por escrito, que es más frío e impersonal
- Explicaciones razonables: Es mejor preguntar si hay una alternativa mejor y justificar por qué vale la pena arreglarlo. A veces los cambios sugeridos pueden parecer muy quisquillosos si no se posee un contexto o una explicación clara.
- Sólo el código: Es muy fácil tomar notas sobre un código durante las revisiones, y más fácil aún sobrepasarse. Hay que centrarse exclusivamente en el código, en el problema que resuelve y no en el desarrollador. Esto reduce la resistencia a las revisiones de código, porque no se trata del desarrollador, se trata de mejorar la calidad del código.
- Sugerencias y arreglos: Es bueno ofrecer sugerencias, sin embargo no todas necesitan ser implementadas ipso facto. Es necesario aclarar cuáles son importantes y tener en cuenta al revisar que cada desarrollador tiene un estilo y su propia forma de resolver una funcionalidad. Las sugerencias deben ser concisas y claras, comentando aquellos puntos clave y que el desarrollador sepa qué hacer tras la revisión.
mentalidad
Como desarrolladores, somos responsables de crear código funcional y mantenible. Puede ser fácil decirlo, aunque no tanto hacerlo debido a los plazos de desarrollo y demás presiones. La refactorización no debería cambiar la funcionalidad, sólo hacer el código más mantenible. Mejorar la mantenibilidad del código puede ser tan importante como arreglar la línea de código que causó un error. Además, es importante mantener una mentalidad abierta durante las revisiones de código. Realizar o acudir a revisiones de código con una actitud defensiva, es malo para todos: para los desarrolladores, el código, el equipo… Si el revisor hace una sugerencia y no hay una respuesta clara de por qué no se debe implementar, lo mejor es realizar el cambio. Si el revisor pregunta sobre una línea de código, puede significar que esa línea confundirá a otros en el futuro. Además, hacer los cambios puede ayudar a revelar problemas de arquitectura o errores más grandes.referencias
Existen diversos libros de referencia sobre el arte de crear código limpio muy reveladores y útiles, éstos son algunos de ellos: Además, también vídeos sobre charlas y consejos útiles sobre las revisiones de código:- All the Small Things by Sandi Metz: Cubre el tema bien, desde la perspectiva de escribir código limpio y reutilizable.
- How to Design a Good API and Why it Matters: Trata la forma en que se construye una aplicación y cómo interactuamos con ella. Muchos de los puntos en el video hablan de temas similares a los aquí tratados.
herramientas
Aunque se pueden hacer revisiones de código sin ningún tipo de software específico, existen algunas herramientas que pueden facilitar esta labor, estas son algunas de ellas:- Upsource: Se trata de un browser para repositorios de código que permite realizar code reviews con la gente de nuestro equipo.
- Crucible: Es una herramienta de revisión de código de la popular Atlassian. Se integra perfectamente con Bitbucket y todo el ecosistema de Atlassian.
- Gerrit: Desarrollada en Google para el desarrollo de Android, es una herramienta de revisión de código y gestión de repositorios Git.
