¿Puede el código comentado ser una documentación valiosa?

79

Escribí el siguiente código:

if (boutique == null) {
    boutique = new Boutique();

    boutique.setSite(site);
    boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
    boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
    boutique.setNom(fluxBoutique.getNom());
    boutique.setSelected(false);
    boutique.setIdWebSC(fluxBoutique.getId());
    boutique.setDateModification(new Date());

    boutiqueDao.persist(boutique);
} else {
    boutique.setSite(site);
    boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
    boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
    boutique.setNom(fluxBoutique.getNom());
    //boutique.setSelected(false);
    boutique.setIdWebSC(fluxBoutique.getId());
    boutique.setDateModification(new Date());

    boutiqueDao.merge(boutique);
}

Aquí hay una línea comentada. Pero creo que hace que el código sea más claro, al hacer evidente cuál es la diferencia entre if y else . La diferencia es aún más notable con el resaltado en color.

¿Puede ser una buena idea comentar un código como este?

    
pregunta Alexis Dufrenoy 11.03.2013 - 10:36

13 respuestas

100

La mayoría de las respuestas se centran en cómo refactorizar este caso específico, pero permítame ofrecer una respuesta general sobre por qué el código comentado es generalmente malo:

Primero, el código comentado no está compilado. Esto es obvio, pero significa que:

  1. Es posible que el código ni siquiera funcione.

  2. Cuando cambian las dependencias del comentario, obviamente no se romperá.

El código comentado es mucho "código muerto". Cuanto más tiempo permanece allí, más se pudre y proporciona cada vez menos valor al siguiente desarrollador.

Segundo, el propósito no está claro. Realmente necesitas un comentario más largo que proporcione un contexto de por qué hay líneas comentadas al azar. Cuando veo solo una línea de código comentada, tengo que investigar cómo llegó allí solo para entender por qué llegó allí. ¿Quien lo escribió? ¿Qué compromiso? ¿Cuál fue el mensaje / contexto de confirmación? Etcétera.

Considera alternativas:

  • Si el objetivo es proporcionar ejemplos de uso de una función / api, proporcione una prueba de unidad. Las pruebas unitarias son códigos reales y se interrumpirán cuando dejen de ser correctas.
  • Si el propósito es preservar una versión anterior del código, use control de código fuente. Prefiero revisar una versión anterior y luego alternar los comentarios a lo largo de la base de código para "revertir" un cambio.
  • Si el propósito es mantener una versión alternativa del mismo código, use el control de código fuente (nuevamente). Para eso son las ramas, después de todo.
  • Si el propósito es aclarar la estructura, considere cómo puede reestructurar el código para hacerlo más obvio. La mayoría de las otras respuestas son buenos ejemplos de cómo podría hacer esto.
respondido por el Chris Pitman 12.03.2013 - 05:25
257

El mayor problema con este código es que duplicaste esas 6 líneas. Una vez que elimines esa duplicación, ese comentario es inútil.

Si creas un método boutiqueDao.mergeOrPersist , puedes reescribirlo como:

if (boutique == null) {
    boutique = new Boutique();
    boutique.setSelected(false);
}

boutique.setSite(site);
boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
boutique.setNom(fluxBoutique.getNom());
boutique.setIdWebSC(fluxBoutique.getId());
boutique.setDateModification(new Date());

boutiqueDao.mergeOrPersist(boutique);

El código que crea o actualiza un determinado objeto es común, por lo que debería resolverlo una vez, por ejemplo, creando un método mergeOrPersist . Ciertamente, no debe duplicar todo el código de asignación para esos dos casos.

Muchos ORM han incorporado soporte para esto de alguna manera. Por ejemplo, podrían crear una nueva fila si el id es cero, y actualizar una fila existente si el id no es cero. La forma exacta depende del ORM en cuestión, y como no estoy familiarizado con la tecnología que está utilizando, no puedo ayudarlo con eso.

Si no desea crear un método mergeOrPersist , debe eliminar la duplicación de alguna otra manera, por ejemplo, introduciendo un indicador isNewBoutique . Puede que no sea bonito, pero es mucho mejor que duplicar toda la lógica de asignación.

bool isNewBoutique = boutique == null;
if (isNewBoutique) {
    boutique = new Boutique();
    boutique.setSelected(false);
}

boutique.setSite(site);
boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE + fluxBoutique.getLogo());
boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE + fluxBoutique.getUrl());
boutique.setNom(fluxBoutique.getNom());
boutique.setIdWebSC(fluxBoutique.getId());
boutique.setDateModification(new Date());

if (isNewBoutique)
    boutiqueDao.persist(boutique);
else
    boutiqueDao.merge(boutique);
    
respondido por el CodesInChaos 11.03.2013 - 10:45
161

Esta es una idea absolutamente horrible . No deja claro cuál es la intención. ¿El desarrollador comentó la línea por error? ¿Para probar algo? ¡¿Qué está pasando ?!

Aparte del hecho de que veo 6 líneas que son absolutamente iguales en ambos casos. Más bien, debe evitar esta duplicación de código. Entonces quedará más claro que en un caso, además, llamará a setSelected.

    
respondido por el JustAnotherUserYouMayKnowOrNot 11.03.2013 - 10:41
117

No, es una idea terrible. Basado en ese fragmento de código, los siguientes pensamientos vienen a mi mente:

  • Esta línea está comentada porque el desarrollador la estaba depurando y olvidó restaurar la línea a su estado anterior
  • Esta línea está comentada porque una vez fue parte de la lógica de negocios, pero ya no es el caso
  • Esta línea está comentada porque causó problemas de rendimiento en la producción y el desarrollador quiso ver cuál fue el impacto en un sistema de producción

Después de ver miles de líneas de código comentado, ahora estoy haciendo lo único sensato cuando lo veo: lo elimino inmediatamente.

No hay ninguna razón sensata para ingresar el código comentado en un repositorio.

Además, tu código usa mucha duplicación. Le sugiero que lo optimice para facilitar la lectura humana tan pronto como sea posible.

    
respondido por el Dibbeke 11.03.2013 - 10:47
49

Simplemente quisiera agregar a la respuesta de CodesInChaos, señalando que puede refactorizarlo aún más en pequeños métodos . Compartir la funcionalidad común por composición evita los condicionales:

function fill(boutique) {    
  boutique.setSite(site);
  boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
  boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
  boutique.setNom(fluxBoutique.getNom());
  boutique.setIdWebSC(fluxBoutique.getId());
  boutique.setDateModification(new Date());
}    

function create() {
  boutique = new Boutique();      
  fill(boutique);
  boutique.setSelected(false);
  return boutiqueDao.persist(boutique);
}

function update(boutique) {
  fill(boutiquie);
  return boutiquieDao.merge(boutique); 
}

function createOrUpdate(boutique) {
  if (boutique == null) {
    return create();
  }
  return update(boutique);  
}
    
respondido por el Alexander Torstling 11.03.2013 - 14:33
23

Si bien este no es un buen caso para el código comentado, hay una situación que creo que lo justifica:

// The following code is obvious but does not work because of <x>
// <offending code>
<uglier answer that actually does work>

Es una advertencia para quienquiera que lo vea más adelante que la mejora obvia no lo es.

Edit: estoy hablando de algo pequeño. Si es grande explica en su lugar.

    
respondido por el Loren Pechtel 12.03.2013 - 04:43
13

En este ejemplo específico, encuentro el código comentado muy ambiguo, en gran parte por las razones descritas en La respuesta de Dibkke . Otros han sugerido formas de refactorizar el código para evitar incluso la tentación de hacer esto, sin embargo, si eso no es posible por alguna razón (por ejemplo, las líneas son similares, pero no lo suficientemente cercanas), agradecería un comentario como:

  

// No es necesario deseleccionar esta boutique, porque [WHATEVER]

Sin embargo, creo que hay algunas situaciones en las que no es reprensible el hecho de dejar (o incluso agregar comentarios). Cuando se utiliza algo como MATLAB o NumPY, a menudo se puede escribir un código equivalente que 1) itera sobre una matriz, procesando un elemento a la vez o 2) opera toda la matriz a la vez. En algunos casos, este último es mucho más rápido, pero también mucho más difícil de leer. Si sustituyo algún código con su equivalente vectorizado, incrustaré el código original en un comentario cercano, como este:

  

%% El siguiente código vectorizado hace esto:

% for ii in 1:N
%    for jj in 1:N
%      etc.
  

% pero la versión de la matriz se ejecuta ~ 15 veces más rápido en una entrada típica (MK, 10/03/2013)

Obviamente, uno debe tener cuidado de que las dos versiones realmente hagan lo mismo y que el comentario se mantenga sincronizado con el código real o se elimine si el código cambia. Obviamente, las advertencias habituales sobre la optimización prematura también se aplican ...

    
respondido por el Matt Krause 11.03.2013 - 17:53
9

La única vez que vi un código comentado fue útil en los archivos de configuración, donde se proporciona el código para cada opción, con muchas opciones comentadas. Esto facilita la activación y desactivación de las funciones simplemente eliminando los marcadores de comentarios.

## import this list of modules
# config.imports = ['os', 'sys']
    
respondido por el Carl Smith 13.03.2013 - 00:09
6

En términos generales, el código solo se documenta a sí mismo a la persona que lo escribió. Si se requiere documentación, escriba la documentación. Es inaceptable esperar que un desarrollador nuevo en una base de código fuente se siente y lea miles de líneas de código para intentar averiguar a partir de un alto nivel qué está sucediendo.

En este caso, el propósito de la línea de código comentada es mostrar la diferencia entre dos instancias de código duplicado. En lugar de intentar documentar sutilmente la diferencia con un comentario, vuelva a escribir el código para que tenga sentido. Luego, si aún siente que es necesario comentar sobre el código, escriba un comentario apropiado.

    
respondido por el Mike Van 11.03.2013 - 15:53
4

No, el código comentado se vuelve obsoleto y pronto es peor que sin valor, a menudo es perjudicial, ya que consolida algunos aspectos de la implementación, junto con todas las suposiciones actuales.

Los comentarios deben incluir detalles de la interfaz y la función prevista; "función deseada": puede incluir, primero intentamos esto, luego intentamos eso, luego fallamos de esta manera.

Los programadores que he visto tratan de dejar las cosas en los comentarios están simplemente enamorados de lo que han escrito, no quieren perderlo, incluso si no está agregando nada al producto terminado.

    
respondido por el Grady Player 11.03.2013 - 19:05
2

Puede ser en casos muy raros, pero no como lo has hecho. Las otras respuestas han explicado bastante bien las razones para ello.

Uno de los casos raros es una plantilla especificación de RPM que usamos en mi tienda como punto de partida para todos los paquetes nuevos, en gran medida para asegurarse de que no se omita nada importante. La mayoría, pero no todos nuestros paquetes tienen un tarball que contiene fuentes que tiene un nombre estándar y se especifica con una etiqueta:

Name:           foomatic
Version:        3.14
 ...
Source0:        %{name}-%{version}.tar.gz

Para paquetes sin fuentes, comentamos la etiqueta y colocamos otro comentario arriba para mantener el formato estándar e indicamos que alguien se ha detenido y ha pensado en el problema como parte del proceso de desarrollo:

Name:           barmatic
Version:        2.71
 ...
# This package has no sources.
# Source0:        %{name}-%{version}.tar.gz

No agrega el código que sabe que no se va a usar porque, como han cubierto otros, podría confundirse con algo que pertenece allí. Puede. sin embargo, sea útil para agregar un comentario que explique por qué falta el código que uno podría esperar que esté allí:

if ( condition ) {
  foo();
  // Under most other circumstances, we would do a bar() here, but
  // we can't because the quux isn't activated yet.  We might call
  // bletch() later to rectify the situation.
  baz();
}
    
respondido por el Blrfl 11.03.2013 - 16:05
0

La aplicación no utiliza el código comentado, por lo que debe ir acompañado de otros comentarios que indiquen por qué no se está utilizando. Pero dentro de ese contexto, hay situaciones en las que el código comentado puede ser útil.

Lo que me viene a la mente es un caso en el que resuelves un problema utilizando un enfoque común y atractivo, pero luego resulta que los requisitos de tu problema real son ligeramente diferentes de ese problema. Especialmente si sus requisitos requieren un código considerablemente mayor, la tentación de los mantenedores de "optimizar" el código utilizando el enfoque anterior probablemente será fuerte, pero al hacerlo solo se recuperará el error. Mantener la implementación "incorrecta" en los comentarios ayudará a disipar esto, ya que puede usarla para ilustrar exactamente el por qué ese enfoque es incorrecto en esta situación.

Esta no es una situación que pueda imaginar que ocurra muy a menudo. Por lo general, debería ser suficiente para explicar las cosas sin incluir una muestra de implementación "incorrecta". Pero puedo imaginar un caso en el que no sea suficiente, por lo que la pregunta es si puede ser útil, sí, puede. Simplemente no la mayoría del tiempo.

    
respondido por el The Spooniest 12.02.2014 - 17:07
-2

Esto no se ve bien amigo.

El código comentado es ... simplemente no código. El código es para la implementación de la lógica. Hacer un código más legible en sí mismo es un arte. Como @CodesInChaos ya ha sugerido que las líneas de código repetitivas no son una buena implementación de la lógica .

¿Realmente crees que un verdadero programador preferirá la legibilidad sobre la implementación lógica? (por cierto, tenemos comentarios y 'complementos' para incluir en nuestra representación lógica).

En lo que a mí respecta, uno debería escribir un código para el compilador y eso es bueno, si 'entiende' ese código. Para la legibilidad humana, los comentarios son buenos, para los desarrolladores (a largo plazo), para las personas que reutilizan ese código (por ejemplo, evaluadores).

De lo contrario, puedes probar algo más flexible aquí, algo como

boutique.setSite (sitio) se puede reemplazar con

setsiteof.boutique (sitio). Hay diferentes aspectos y perspectivas de la programación orientada a objetos a través de los cuales puede aumentar la legibilidad.

Aunque este código parece ser muy atractivo al principio y uno puede pensar que ha encontrado un camino para la legibilidad humana, mientras que el compilador también hace su trabajo a la perfección, y todos comenzamos a seguir esta práctica, nos llevará a un archivo borroso que se vuelve menos legible en el tiempo y más complejo, ya que se expandirá hacia abajo.

    
respondido por el Aura 11.03.2013 - 13:36

Lea otras preguntas en las etiquetas