Ésta es una traducción y adaptación del artículo titulado «Code Review Best Practices», escrito y publicado originalmente por Kenvin London el 5 de Mayo de 2015.
En Wiredrive realizamos bastantes revisiones de código. Nunca había hecho una antes de comenzar a trabajar aquí, así que fue una nueva experiencia. Creo que es una buena idea cristalizar algunas de las cosas en las que me enfoco cuando hago revisiones de código, hablando sobre la mejor forma que he encontrado para acercarme a ellas.
Rápidamente, una revisión de código es una discusión entre dos o más desarrolladores a cerca de cambios en el código para solucionar un problema o agregar una nueva característica.
¿Qué observar al hacer una revisión?
Arquitectura / Diseño
- Principio de la responsabilidad única: La idea de que una clase debe tener una y sólo una responsabilidad, es más difícil de lo que uno podría esperar. Usualmente también aplico éste principio a los métodos: si se tiene que usar una conjunción «y» para terminar de describir lo que un método es capaz de hacer, puede tratarse de un mal nivel de abstracción.
- Principio abierto/cerrado: Si el lenguaje usado es orientado a objetos, ¿los objetos pueden ser extendidos pero están protegidos para no ser modificados? ¿Qué pasa si necesitamos agregar un atributo, función o pieza de código?
- Duplicación de código: Prefiero seguir la regla de los tres strikes. Si el código está replicado una vez, usualmente está bien aunque no me agrade. Si está replicado más de una vez, debe hacerse una refactorización para separar las funciones comunes.
- Prueba de los ojos entornados: Si entorno los ojos, ¿la forma de éste código se ve idéntica a otras? ¿Hay patrones que pueden indicar otros problemas en la estructura del código?
- Código en un mejor estado: Si estoy cambiando un área desordenada del código, es tentador agregar un par de líneas y dejarlo así. Recomiendo ir un paso más allá y organizar el código para dejarlo mejor a cómo lo encontré.
- Errores potenciales: ¿Hay errores de diferencia por un paso? ¿Los ciclos terminan en la forma que se espera? ¿A caso terminarán?
- Eficiencia: Si se ha implementado un algoritmo en el código, ¿se está usando una implementación eficiente? Por ejemplo, iterar sobre una lista de llaves en un diccionario es una forma ineficiente de buscar el valor deseado.
Estilo
- Nombres de los métodos: Darle nombre a las cosas es uno de los problemas difíciles en ciencias de la computación. Si un método es llamado `obtener_nombre_del_mensaje_en_la_fila` y en realidad está hace algo completamente diferente, como sanear el HTML de una entrada, entonces no es un nombre descriptivo para el método y causará confusión.
- Nombres de variables: `foo` y `bar` probablemente no son nombres útiles para estructuras de datos. De forma similar, `e` no es tan útil comparado con `excepcion`. Se tan explicito como necesites (dependiendo del lenguaje). Nombres de variable expresivos hacen más fácil entender el código cuando se tiene que volver a leer.
- Longitud de una función: Mi regla del pulgar es que una función debe tener menos de 20 líneas, aproximadamente. Si veo un método de más de 50 líneas, siento que lo mejor es dividirlo en partes más pequeñas.
- Longitud de una clase: Creo que las clases deben tener menos de 300 líneas en total, e idealmente menos de 100. Es usual que clases muy largas puedan ser separadas en objetos diferentes, lo que hace más fácil entender el propósito de cada clase.
- Longitud de un archivo: Es algo variable, dependiendo del lenguaje, pero para lenguajes dinámicos como Python, creo que alrededor de 1,000 líneas de código es lo máximo que deberíamos tener en un archivo. En caso de ser más grande, es una buena señal de que el contenido debe ser dividido en archivos más pequeños y enfocados. Mientras más grande sea un archivo, es más difícil encontrar información.
- Documentación en comentarios: Para métodos complejos, los que su funcionamiento no es obvio o aquellos con una larga lista de argumentos, ¿hay un comentario (docstring) explicando la función de cada argumento?
- Código comentado: Es una buena idea remover código que ya no se utiliza y que se ha dejado cómo comentarios.
- Número de argumentos en un método: Para métodos y funciones, ¿tienen 3 o menos argumentos? Más de 3 probablemente significa que pueden ser agrupados de otra forma.
- Facilidad de lectura: ¿El código es fácil de entender? ¿Tienen que detenerte frecuentemente durante la revisión para descifrarlo?
Pruebas
- Cobertura de las pruebas: Me gusta ver pruebas para nuevas características. ¿La pruebas indican que se pensó en ellas antes de escribirlas? ¿Cubren los casos en que habría fallos? ¿Qué tan frágiles son? ¿Qué tan amplias son? ¿Son lentas?
- Pruebas en el nivel correcto: Cuando reviso las pruebas también me aseguro que estén probando el nivel correcto. En otras palabras, ¿están al nivel suficientemente bajo para corroborar la funcionalidad esperada? El desarrollador Gary Bernhardt (Destroy All Software) recomienda una relación de 95% de pruebas unitarias con un 5% de pruebas de integración. Me he dado cuenta que en proyectos con Django, es fácil crear pruebas de alto nivel por accidente, terminando con un conjunto de pruebas lento, así que es importante mantenerse alerta.
- Número de objetos simulados: Crear objetos simulados (mocks, dobles) es genial, pero simular todo no lo es. Como regla del pulgar, si hay más de tres objetos simulados en una prueba, el código debe ser reestructurado. Puede ser que la prueba sea demasiado amplia o que la función que evalúa sea demasiado grande. Tal vez no necesite una prueba a nivel unitario y con una prueba de integración sea suficiente. De cualquier forma, es algo que se debe discutir.
- Cobertura de los requerimientos: Usualmente, como la parte final de una revisión, observo los requerimientos de la historia, tarea o bug que dio origen al código. Si no cubre alguno de los requerimientos, es mejor no aceptar el código antes de que sea enviado a QA.
Revisa tu propio código
- ¿Dejé un comentario o TODO en los cambios?
- ¿Los nombres de las variables tienen sentido?
- ...y alguna violación a lo discutido en puntos anteriores.
Quiero estar seguro de que pasaría mi propia revisión de código antes de pedirle a otras personas que lo revisen. También, duele menos recibir comentarios de uno mismo que de otros.
Manejando las revisiones de otros
- Haz preguntas: ¿Cómo funciona ese método? ¿Si éste requerimiento cambia, qué más tendría que cambiar? ¿Cómo podemos hacer esto más fácil de mantener?
- Refuerza y felicita las buenas prácticas: Una de las partes más importantes de la revisión de código es recompensar a los desarrolladores por su crecimiento y esfuerzo. Pocas cosas se sienten mejor que recibir el reconocimiento de un compañero. Yo trato de ofrecer la mayor cantidad posible de comentarios positivos.
- Obtén más detalles en persona: En ocasiones, un cambio en la arquitectura puede ser muy grande y es más fácil discutirlo en persona que a través de comentarios. De forma similar, si la discusión de un punto genera comentarios de ida y vuelta, con frecuencia prefiero retomarlo en persona para finalizar la discusión.
- Explica el razonamiento: He encontrado que es mejor preguntar si hay alguna alternativa superior y justificar porque es importante arreglarlo. Algunas veces puede haber la sensación de que los cambios sugeridos son superficiales si no hay algún contexto o explicación.
- Mantén el enfoque en el código: Es fácil tomar de forma personal los comentarios en las revisiones de código, especialmente si nos enorgullecemos de nuestro propio trabajo. No hay que olvidar que las discusiones son acerca de cómo mejorar la calidad del código y no son específicos a los desarrolladores. Esto disminuye la resistencia.
- Haz notar los cambios importantes: Es común que ofrezca varias sugerencias, pero no es necesario actuar en todas. Aclarar si es importante arreglar un punto antes de considerarlo terminado es útil tanto para quien revisa como para quien somete el código a revisión. Hace los resultados de la revisión claros y de valor práctico.
En cuanto a la mentalidad
Además, recuerda mantener una mente abierta durante las revisiones. Creo que es algo con lo que la mayoría tenemos problemas. Uno puede ponerse a la defensiva porque puede tomarlo personal cuando alguien te dice que tu código podría ser mejor.
Si la persona que revisa el código hace una sugerencia y no tengo una respuesta clara de porque esa sugerencia no debería ser implementada, usualmente hago el cambio. Si me hacen una pregunta acerca de una línea de código, quiere decir que podría confundir a otros desarrolladores en el futuro. Otro beneficio, es que el hacer los cambios puede revelar otros problemas de arquitectura u otros bugs.
(Gracias a Zach Schipono por recomendar que se agregará ésta sección.)
Atención a los cambios sugeridos
Referencias adicionales
- Código Limpio: Manual de estilo para el desarrollo ágil de software de Robert C. Martin
- Refactoring: Improving the Design of Existing Code de Martin Fowler (sólo disponible en inglés).
Son un gran fanático de las charlas, así que aquí algunas que relacionadas que considero útiles y que recordé mientras escribía ésta entrada:
- All the Small Things por Sandy Metz: Cubre bien el tema, particularmente desde la perspectiva de escribir código limpio y reutilizable. (Charla en inglés.)
- How to Design a Good API and Why it Matters de Joshua Bloch: Una API en el sentido y con el significado de ser la forma en que una aplicación es construida y cómo interactuamos con ella. Varios de los puntos del vídeo hablan de temas similares a los discutidos en éste documento. (Charla en inglés.)
Discusión en Hacker News.
No hay comentarios.:
Publicar un comentario