Donde trabajo, comenzamos hace poco a formalizar las revisiones de código. Esperábamos que fuera algo bastante polémico dentro del grupo de desarrolladores, aunque no resultó así. Antes de comenzar a trabajar en el sistema de revisiones de código, leímos bastante sobre el tema y tomamos los recaudos que nos parecían necesarios.
El tema de las revisiones en nuestro grupo ha surgido como un complemento a la implementación de TDD.
Uno de los problemas más importantes es poder controlar la calidad del código que se genera para satisfacer las necesidades funcionales. Esa es una de las razones por las que tomamos la iniciativa de implementar Integración contínua, Checkstyle, PMD, Cobertura, TDD y las revisiones de código.
Sin objeción, las revisiones de código son necesarias. Uno puede argumentar que con realizar tests funcionales sería suficiente, pero lamentablemente que un código cumpla su cometido funcional no garantiza que cumpla su cometido no-funcional, es decir, no garantiza que sea escalable, mantenible, extensible, o incluso... legible.
Igualmente, después de un tiempo de hacerlas, nos encontramos con el primer problema ¿hasta dónde se debe llegar con con las revisiones de código? Nuestra experiencia es que las revisiones de código demasiado estrictas pueden ser muy contraproducentes. Es muy fácil que se vean como un "aquí vienen los arquitectos a hacer de policías y hacernos la vida mucho más difícil". Con las revisiones de código es muy, pero que muy fácil caer en el autoritarismo y afectar en la moral de los desarrolladores que creerán limitada su capacidad de creación.
Algunos argumentarán que cortar la capacidad de creación de los programadores es algo bueno, no lo creemos así. Es la típica aproximación de factoría de software. Tener personas robotizadas programadas para hacer lo que se le ha dicho exactamente del modo que se le ha dicho. Bajo esta suposición, el establecer revisiones de código muy estrictas sería el mejor modo de garantizar la calidad. Quizás se logre esa calidad, pero lamentablemente sería el peor modo de garantizar la continuidad del personal. Al cortar la creatividad, no sólo se le esta prohibiendo a los desarrolladores hacer lo que más les gusta, si no que además éstos perderán toda "enlace sentimental" con el código que han creado. No esperes que estos desarrolladores vuelvan atrás, o se queden un par de horas, o piensen en casa como mejorar un código que les han obligado a escribir.
El mejor sistema de revisiones de código será aquel que permita un equilibrio entre los deseos del arquitecto y las ambiciones del desarrollador. Hay una serie de puntos que pueden hacer llegar a este equilibrio:
- Utilizar herramientas de análisis estático de código: Herramientas como PMD o Checkstyle ayudarán a encontrar errores de código básicos, sin tener que advertir a los programadores que han cometido errores. El estar continuamente advirtiendo personalmente a los desarrolladores de errores que han cometido es algo que puede ser incómodo, y en algunos casos causar problemas sociales ("ahí viene el sabelotodo a decirme lo que ya sabía"). Para ello, nada mejor que introducir estas herramientas junto con la integración continua y realizar un seguimiento a nivel de SLA (niveles de servicio) de como queremos que las métricas de estas herramientas afecten a la calidad del código. Nosotros trabajamos con Scrum y como parte de la definición de terminado están las métricas de Checkstyle, PMD, Cobertura, entre otras.
- Centrarse en los aspectos no funcionales: Las revisiones de código no deberían centrarse en si el código funciona, o no, ya que eso es algo que desvelarán los tests de integración.
- No olvidar el componente social de las revisiones: Este es un aspecto muy importante y el que mejor manejamos hasta el momento. Las revisiones no son un sistema para mostrar que el revisor sabe más, o tiene más autoridad que el revisado. Tampoco son un sistema para asegurarnos de que las cosas se han hecho como hemos ordenado. Es muy importante que las revisiones sean un mecanismo de aprendizaje mutuo. Si las cosas no se han hecho como se recomendaban, seguramente haya razones para ello, que en su caso pueden ser de peso; en caso contrario el revisor debe actuar como coach y aconsejar, o incluso ayudar realizando pair programming si es necesario.
- Olvidar los aspectos superfluos: Muchas veces, las revisiones tienden a derivar hacia temas bastante superfluos como el "porque esta variable es protected en vez de privada", o "por qué este valor es estático", o "por qué utilizas un espaciado de 6 si nuestro estándar dice que hay que utilizar 4". La mayor parte de estas cosas son realmente superfluas. Algunas se detectan con análisis estático, otras se solucionan con un formateador de código, etc. El valor de las revisiones no reside en estas minucias, sino en detectar el impacto de los nuevos componentes en la arquitectura del sistema. ¿Es escalable? ¿Es fácilmente mantenible? ¿Lo podremos monitorizar? ¿Está haciendo auditoría? ¿Se ajusta a nuestro modelo de componentes? ¿Define interfaces como el resto de nuestros componentes? ¿Está haciendo buen uso de los frameworks? ¿Hay problemas de rendimiento, memory leaks, o faltas graves claramente visibles?
- Dar libertad al desarrollador: Al hilo del punto anterior, una vez revisados los aspectos no funcionales y el impacto en la arquitectura, sinceramente, queda poco por revisar. Darle libertad al desarrollador para que cree sus propios componentes es importante, incluso darle la libertad para mejorar sus creaciones (dentro de un órden) es muy valioso ya que crea vínculos morales entre el desarrollador y sus "criaturas". De hecho, es importante sentarse e interesarse por el cómo ha sido diseñado el componente, y realizar sugerencias en caso de que sea necesario. A fin de cuentas, ¡a los programadores nos encanta hablar de lo que programamos!
Entonces como conclusión, revisiones de código SI, con paciencia y desde un punto de vista didáctico para ambos, revisor y revisado. La revisión de código siempre tiene que encararse como un proceso de aprendizaje, una oportunidad para que todos los involucrados aprendan y crezcan profesionalmente. Este debe ser el objetivo último de toda revisión.