martes, febrero 12, 2008

Code reviews, ¿buena o mala idea?

martes, febrero 12, 2008 por Martín


Hace poco que en mi empresa un compañero se encargó de preparar un sistema de code reviews. Como era de esperar, ha sido algo bastante polémico dentro del seno de los desarrolladores. Esto me ha hecho pensar bastante sobre el tema.

Uno de los problemas más importantes de las empresas es controlar la calidad del código que se genera para satisfacer las necesidades funcionales. Esa es una de las razones por las que yo mismo recomendé el realizar revisiones de código. Innegablemente, 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.

Sin embargo, ¿hasta dónde se debe llegar con con las revisiones de código? Mi experiencia es que las revisiones de código demasiado estrictas pueden ser muy contraproducentes según como sea la estructura de desarrollo de una empresa. 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 verán limitada su capacidad de creación.

Algunos argumentarán que cortar la capacidad de creación de los programadores es algo bueno. 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. Seguramente lo sería, 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 code reviews 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 Findbugs 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 peresonalmente 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 listillo a decirme lo que ya sabía"). Para ello, nada mejor que introducir estas herramientas en la build y despreocuparse de los fallos triviales.
  • 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. 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 mentor 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!


Resumiendo, code reviews si, pero con paciencia, calma, y desde un punto de vista didáctico. Todo el mundo puede aprender de las reviews, tanto revisores como revisados.