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

83

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);
}

Hay una línea comentada aquí. Pero creo que aclara el código al hacer obvio cuál es la diferencia entre ify else. La diferencia es aún más notable con el resaltado de color.

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

Alexis Dufrenoy
fuente

Respuestas:

109

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

Primero, el código comentado no se compila. Esto es obvio, pero significa que:

  1. El código podría incluso no funcionar.

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

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

En segundo lugar, el propósito no está claro. Realmente necesita un comentario más largo que proporcione el 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? Etcetera

Considere alternativas:

  • Si el objetivo es proporcionar ejemplos del uso de una función / api, proporcione una prueba unitaria. Las pruebas unitarias son código real y se romperán cuando ya no sean correctas.
  • Si el propósito es preservar una versión anterior del código, use el control de origen. Prefiero verificar una versión anterior y luego alternar los comentarios en toda 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 origen (nuevamente). Para eso están 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.
Chris Pitman
fuente
55
Creo que le falta una razón importante: Documentación: si el propósito es documentar opciones de diseño alternativas, se debe proporcionar una explicación de la alternativa y, especialmente, la razón por la que se ha descartado en lugar del código original.
Sarien
14
Las opciones de diseño se explican mejor en un lenguaje humano que en un lenguaje de programación.
Mark E. Haase
3
¿Cómo sería posible que los desarrolladores posteriores que se hagan cargo de mi proyecto sepan que existe una implementación alternativa / anterior / fallida en el control de origen? ¿Se espera que los nuevos desarrolladores revisen todos los historiales de versiones y cambien los registros? ¿O es una práctica común usar el comentario para vincular al hash de un commit anterior para cada implementación alternativa útil? Si es así, nunca me di cuenta.
Moobie
Sin embargo, hay una advertencia para esto. A veces, dos enfoques de código equivalentes pueden diferir en rendimiento y confiabilidad de manera que uno sea eficiente y el otro sea legible. En tal caso, es aceptable usar la variante de rendimiento, pero ponga la variante legible en los comentarios para que sea más fácil entender el propósito del código. A veces, una línea de código (comentada) puede ser más clara que una explicación detallada.
Flater
263

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

Si crea un boutiqueDao.mergeOrPersistmétodo, puede volver a escribir esto 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 debe resolverlo una vez, por ejemplo, creando un mergeOrPersistmétodo. 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 ides cero y actualizar una fila existente si idno 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 mergeOrPersistmétodo, debe eliminar la duplicación de alguna otra manera, por ejemplo, mediante la introducción de una isNewBoutiquebandera. Puede que no sea bonito, pero sigue siendo 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);
CodesInChaos
fuente
166

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? ¡¿Que esta pasando?!

Aparte del hecho de que veo 6 líneas que son absolutamente iguales en ambos casos. Por el contrario, debe evitar la duplicación de este código. Entonces será más claro que, en un caso, también se llama a setSelected.

JustAnotherUserYouMayKnowOrNot
fuente
99
Convenido. Supongo que ver que la línea comentada es un comportamiento antiguo que se ha eliminado. Si se necesita un comentario, debe estar en lenguaje natural, no en código.
Jules
44
Estoy completamente de acuerdo! Recientemente he pasado horas enteras tratando de comprender y limpiar algunas aplicaciones que heredé que son casi completamente ilegibles debido a esta práctica. ¡También incluye código que se ha desconectado de todos los demás códigos pero no se ha eliminado! Creo que este es un propósito principal detrás de los sistemas de control de versiones. Tiene comentarios, así como los cambios que los acompañan. Al final, me han agregado al menos 2 semanas de trabajo a mi plato en gran parte debido a esta práctica.
bsara
punto de vista similar en esta publicación: No contamine la base de código con código comentado
Nick Alexeev
120

No, es una idea terrible. Basado en ese 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 se comenta porque una vez fue parte de la lógica de negocios, pero ya no es el caso
  • Esta línea se comenta porque causó problemas de rendimiento en la producción y el desarrollador quería ver cuál era 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 de inmediato.

No hay una razón razonable para registrar el código comentado en un repositorio.

Además, su código usa mucha duplicación. Le sugiero que optimice eso para la legibilidad humana lo antes posible.

Dibbeke
fuente
1
Sin embargo, me deshago del código duplicado, creo que difícilmente puede verse como una optimización.
Alexis Dufrenoy
23
Es una optimización para la legibilidad humana
jk.
11
@Traroth puede optimizar la velocidad, el uso de la memoria, el consumo de energía o cualquier otra métrica, por lo que no veo que no pueda optimizar la legibilidad (aunque como métrica es un poco más flexible)
jk.
3
De hecho, me refería a la legibilidad humana. Pequeña pista aquí: su responsabilidad más importante en la programación es su código. Entonces, menos es realmente más aquí.
Dibbeke
44
El software como responsabilidad también se trata en c2.com/cgi/wiki?SoftwareAsLiability Desde allí: "Producir más código no siempre es una ganancia. El código es costoso de probar y mantener, por lo que si se puede hacer el mismo trabajo con menos código que es una ventaja. No comente el código muerto, simplemente elimínelo. El código comentado se vuelve obsoleto e inútil muy rápido, por lo que también puede eliminarlo lo antes posible para perder el desorden. Mantenga buenas copias de seguridad para que sea más fácil ".
ninjalj
51

Solo me gustaría 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);  
}
Alexander Torstling
fuente
66
Creo que esa es la sugerencia más limpia aquí.
Alexis Dufrenoy
+1, y también agregaría que cuanto más evite pasar nullobjetos, mejor (creo que esta solución es un buen ejemplo).
Nadir Sampaoli
Me gustaría pasar boutiqueDaocomo entradas a createy update.
Happy Green Kid Naps
¿Cómo puede funcionar esto? ¿Cómo puede saber cuándo llamar a crear y cuándo llamar a actualizar? El código original analiza la boutique y sabe si necesita actualizarse o crearse. Esto simplemente no hace nada hasta que llame crear o actualizar ...
Lyrion
Lyrion: Trivial, también agregaré ese código para mayor claridad.
Alexander Torstling
27

Si bien esto claramente 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 quien lo vea más tarde que la mejora obvia no lo es.

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

Loren Pechtel
fuente
55
¿Qué tiene de malo // the following part done like it is because of X? Explicar por qué usted hizo algo la manera que lo hizo, no por qué lo hizo , no a él de alguna determinada manera. En su ejemplo particular, elimina la necesidad de un bloque potencialmente grande de código comentado por completo. (No voté en contra, pero ciertamente puedo ver por qué esto sería rechazado.)
un CVn
13
Michael, ya que hace más clara a otros codificadores (y usted mismo días / semanas / meses posteriores) que sí, que se trate de que el limpiador de aproximación / más inteligente, pero no, no funcionó debido a X, por lo que no debe molestarse en intentarlo de nuevo. Creo que este es un enfoque completamente legítimo y votó esta respuesta tristemente enterrada.
Garrett Albright
1
@GarrettAlbright: Gracias, me alegra ver que alguien lo entiende.
Loren Pechtel
3
@LorenPechtel: No solo eso, estaba a punto de escribir más o menos exactamente lo mismo. Hay situaciones en las que es muy, muy útil saber rápidamente qué soluciones "obvias" ya se han probado sin éxito y por qué no funcionan.
JensG
3
Además del código fallido con explicación, también comentaría implementaciones alternativas del código que podrían ser más eficientes en un entorno de producción diferente. Por ejemplo, he codificado una versión de tiempo exponencial directa de un algoritmo y una versión de tiempo polinomial complejo. Pero en la producción actual, nes pequeño, y el algo exponencial es mucho más rápido. Si de alguna manera ncambia más adelante, ¿cómo podría un futuro desarrollador siguiendo mi proyecto conocer una implementación diferente del código enterrado en los cientos de confirmaciones en el control de código fuente?
Moobie
14

En este ejemplo específico, encuentro que el código comentado es muy ambiguo, en gran parte por las razones descritas en la respuesta de Dibkke . Otros han sugerido formas en que podría refactorizar el código para evitar incluso la tentación de hacerlo, 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 anular la selección de esta boutique porque [LO QUE]

Sin embargo, creo que hay algunas situaciones en las que dejar el código (o incluso agregar comentarios) no es reprensible. Cuando se usa algo como MATLAB o NumPY, a menudo se puede escribir 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 reemplazo algún código con su equivalente vectorizado, inserto 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 matriz se ejecuta ~ 15 veces más rápido en la entrada típica (MK, 03/10/2013)

Obviamente, uno debe tener cuidado de que las dos versiones realmente hagan lo mismo y que el comentario permanezca 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 ...

Matt Krause
fuente
"Obviamente, uno debe tener cuidado de que las dos versiones realmente hagan lo mismo y que el comentario permanezca sincronizado con ..." - allí mismo usted explicó por qué no es una buena idea.
sleske
1
Bueno, eso es un problema con todos los comentarios, ¿verdad? Algunos códigos vectorizados son lo suficientemente opacos como para que los comentarios valgan la pena, y tener una versión "desenrollada" puede ser útil para la depuración.
Matt Krause
Cierto. Aún así, trataría de mantener el comentario lo más breve posible, no usar el código fuente completo. De todos modos, si tienes un ejemplo, preguntar cómo hacer que sea legible sería una buena pregunta (aquí o en codereview.se).
sleske
1
En su último caso, mantendría ambas variantes de código como código compilable.
CodesInChaos
12

La única vez que vi un código comentado que fue útil fue en los archivos de configuración, donde se proporciona el código para cada opción, pero comentado, lo que hace que sea fácil habilitar la configuración simplemente eliminando los marcadores de comentarios:

## Enable support for mouse input:
# enable_mouse = true

En este caso, el código comentado ayuda a documentar todas las opciones disponibles y cómo usarlas. También es convencional usar los valores predeterminados en todas partes, por lo que el código también documenta la configuración predeterminada.

Carl Smith
fuente
7

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 a leer miles de líneas de código para intentar descubrir desde un alto nivel lo que 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, reescribe el código para que tenga sentido. Luego, si todavía siente que es necesario comentar sobre el código, escriba un comentario apropiado.

Mike Van
fuente
2
Esto suena bastante cierto. Mucha gente (incluido yo mismo) piensa que su código es tan increíble que no necesita documentación. Sin embargo, todos los demás en el mundo lo encuentran engulle a menos que esté completamente documentado y comentado.
Ryan Amos
"el código solo se documenta por sí solo a la persona que lo escribió " - Elija un fragmento de código complejo y sin comentarios que escribió hace un año e intente comprenderlo en un tiempo limitado. No puedes? Ooops
JensG
Creo que es un poco más matizado. Una gran cantidad de código bien escrito es inteligible y puede entenderse sin comentarios. El problema es tratar de averiguar el panorama general (incluso a un nivel bastante local) cuando solo tiene detalles intrincados para continuar. Los comentarios son buenos para explicar fragmentos de código no obvios, pero cuando tiene buenas cadenas de documentos, explicando para qué es realmente cada función, clase y módulo, necesita mucha menos ayuda para dar sentido a la implementación.
Carl Smith
4

No, el código comentado se vuelve obsoleto, y pronto es peor que inútil, a menudo es dañino, ya que consolida algunos aspectos de la implementación, junto con todos los supuestos actuales.

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

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

Grady Player
fuente
2

Puede ser en casos muy raros, pero no como lo ha hecho. Las otras respuestas han explicado bastante bien las razones de eso.

Uno de los casos raros es una especificación de RPM de plantilla que usamos en mi tienda como punto de partida para todos los paquetes nuevos, en gran medida para asegurarnos de que no quede 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 los paquetes sin fuentes, comentamos la etiqueta y colocamos otro comentario encima para mantener el formato estándar e indicar que alguien se detuvo y pensó 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 código que sabe que no se usará porque, como lo 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 esperaría:

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();
}
Blrfl
fuente
55
sin embargo, ese comentario no es un código comentado.
jk.
1
@jk: Puede decirse que tiene razón.
Blrfl
1

La aplicación no utiliza el código comentado, por lo que debe ir acompañado de más comentarios que indiquen por qué no se está utilizando. Pero dentro de ese contexto, no son situaciones donde el código de salida comentado puede ser de utilidad.

Lo que me viene a la mente es un caso en el que resuelve un problema utilizando un enfoque común y atractivo, pero luego resulta que los requisitos de su problema real son ligeramente diferentes de ese problema. Especialmente si sus requisitos requieren mucho más código, la tentación de los encargados de "optimizar" el código utilizando el enfoque anterior probablemente será fuerte, pero hacerlo solo traerá de vuelta el error. Mantener la implementación "incorrecta" en los comentarios ayudará a disipar esto, porque puede usarla para ilustrar exactamente 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 explicar las cosas sin incluir una implementación de muestra "incorrecta". Pero puedo imaginar un caso en el que eso no es suficiente, por lo tanto, dado que la pregunta es si puede ser útil, sí, puede serlo. Simplemente no la mayor parte del tiempo.

El más cuchara
fuente
1
Lo siento, pero no veo ningún valor del código de comentario. El código que está comentado no se usa, por lo tanto, no tiene lugar en el código de producción.
Vladimir Kocjancic
1
Defina "usado".
JensG
Creo que quiso decir "ejecutado"
Alexis Dufrenoy
-2

Esto no se ve bien amigo.

El código comentado es ... simplemente no es 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 muy 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 poner 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, probadores).

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

boutique.setSite (sitio) se puede reemplazar con

setsiteof.boutique (sitio). Hay diferentes aspectos y perspectivas de OOP a través de los cuales puede aumentar la legibilidad.

Si bien este código parece ser muy atractivo al principio y uno puede pensar que ha encontrado una forma de legibilidad humana, mientras que el compilador también hace su trabajo perfectamente, y todos comenzamos a seguir esta práctica, dará lugar a un archivo borroso que será menos legible a tiempo y más complejo, ya que se expandirá hacia abajo.

Aura
fuente
15
"En lo que a mí respecta, uno debería escribir un código para el compilador" Oh, por favor, no lo hagas. Así es como terminas con monstruosidades que parecen que podrían tomarse directamente del concurso de ofuscado C y similares. Las computadoras son binarias, mientras que los humanos usan una lógica difusa (por cierto, esto es doble para los dueños de mascotas). El tiempo de la computadora es casi gratis hoy (básicamente solo el uso de electricidad) mientras que el tiempo del programador es comparativamente muy costoso. Escriba código para humanos y el compilador lo entenderá. Escriba el código para el compilador sin tener en cuenta a los humanos, y no hará muchos amigos en el equipo.
un CVn
3
" escribir código para un compilador " - En realidad no lo haces. La persona que debe tener en mente es la que le entregó la tarea de mantener su código.
JensG