Code Smell: Comentarios

El valor de los comentarios es una de las guerras religiosas más antiguas de la programación. Por eso, cuando en una charla o un artículo se menciona a los comentarios como un Code Smell (y nada menos que uno de los citados en la lista original de smells) se suele armar una linda discusión.

¿Cómo puede ser que algo que nos vienen inculcando desde que empezamos a programar sea negativo? ¿Puede ser que todas esas veces que nos negamos a comentar nuestro código teníamos razón? Bueno, para ser exactos lo que dicen Fowler y Beck no es que los comentarios sean un smell en si mismos si no que se utilizan para disimular smells. Es decir, tenemos código que no es tan bueno como debería y en vez de mejorarlo lo comentamos. Como dice Bob Martín en su libro Clean Code, cuando usamos comentarios lo que estamos haciendo es admitir que fallamos en producir código lo suficientemente claro como para no necesitarlos.

Veamos algunos ejemplos de comentarios que se utilizan normalmente y como hacerlos innecesarios y eliminarlos hace mejor nuestro código

1. Comentario obvio

 class Route {
  int id; //the id of this route
 }

¿Y que otra cosa va a ser el field id de una clase Route? Sin embargo, este tipo de comentarios son muy comunes, normalmente porque algún standard corporativo exige que todos los campos tengan un comentario descriptivo.

Hay casos más estúpidos todavía (y juro que he visto esto en la realidad):

 i++; //Increments i

2. Comentario reemplazable por código

 const int MAX_QUEUE = 5; //Max number of queue per process

Aquí el comentario tiene más información que el código, pero sería mucho mejor escribirlo directamente así:

 const int MAX_QUEUE_PER_PROCESS = 5;

Consiguiendo de esta manera documentar no solo la declaración si no el uso de la constante. Otro ejemplo:

 //verify if the request is a cancelation
 if(request.equals(Constants.CANCEL_SMS_JOB) ||
  request.equals(Constants.CANCEL_MMS_JOB) ||
  request.equals(Constants.CANCEL_WAPPUSH_JOB)) {
  //Some code
 }

Lo cual sería mucho más claro si se escribiera asi

 if(request.isACancelation()) {
  //Some code
 }


 boolean isACancelation() {
  return equals(Constants.CANCEL_SMS_JOB) ||
    equals(Constants.CANCEL_MMS_JOB) ||
    equals(Constants.CANCEL_WAPPUSH_JOB);
 }

Además de ser más claro, esta alternativa brinda la posibilidad de reutilizar la lógica en caso de ser necesario.

3. Comentarios históricos

 //2010-09-01 George Creation
 //2010-09-03 John   Add edit funcionality

¿Cuál es el problema? ¿Tu SCM ya no funciona?

4. Trackeo de posición en el codigo

 while(condition1) {
  if(condition2) {
  } else { // end if condition2
  // ...
  // lots and lots of pre...
  // ...
  } //end else conditionN
 } //end while condition1

Este tipo de comentarios parecen totalmente lógicos y de hecho ayudan a guiarse en código complicado. Sin embargo, no son una solución y están enmascarando un problema más grave: el código es demasiado complejo y el método es innecesariamente largo. La verdadera solución al problema de fondo es partir el método en varios métodos cortos y no tener bajo ninguna circunstancia estructuras lógicas que no sean visibles en su totalidad en una pantalla.

Con estos ejemplos no queremos decir que todos los comentarios son inútiles. Pero en general son sospechosos y vale la pena tener siempre en mente la frase de Kent Beck: nunca comentes código de mala calidad, refactorizalo y hacelo excelente.