Maintenance du code: ajouter des commentaires dans le code ou tout simplement laisser le contrôle de version?

42

Nous avons été invités à ajouter des commentaires avec les balises de début, de fin, la description, la solution, etc. pour chaque modification que nous apportons au code dans le cadre de la correction d'un bogue / de la mise en œuvre d'un CR.

Mon souci est de savoir si cela apporte une valeur ajoutée? En l’état actuel, nous avons tous les détails dans l’historique de contrôle des versions, ce qui nous aidera à suivre chaque changement.

Mais mes responsables insistent pour que les commentaires soient une "bonne" pratique de programmation. L'un de leurs arguments est lorsqu'un CR doit être réduit / modifié, il serait fastidieux de ne pas avoir de commentaires.

Étant donné que les modifications se feraient en grande partie entre les codes, cela aiderait-il vraiment d'ajouter des commentaires pour chaque modification que nous apportons? Ne devrions-nous pas laisser cela au contrôle de version?

Chillax
la source

Réponses:

43

Tu as tout à fait raison. Le suivi des modifications est la tâche de votre système de contrôle de version. Chaque fois que vous effectuez un commit, vous devez écrire un message expliquant ce qui a été fait, et référencer votre système de suivi des bogues s'il s'agit d'un correctif. Mettre un commentaire dans le code en disant

// begin fix for bug XXXXX on 10/9/2012
...
// end fix for bug XXXXX

chaque fois que vous corrigez un bogue, votre code sera rapidement illisible et incontrôlable. Il en résultera également la duplication de la même information à deux endroits, ce qui aggravera les dégâts.

Les commentaires ne doivent pas être utilisés pour le suivi des bogues et ils ne doivent pas non plus décrire ce que fait votre code. Ils devraient expliquer pourquoi vous faites X ou pourquoi vous faites X de cette manière particulière. Si vous ressentez le besoin d'écrire un commentaire expliquant ce qu'un bloc de code est en train de faire, c'est une odeur de code qui indique que vous devez refactoriser ce bloc en une fonction avec un nom descriptif.

Donc au lieu de

// fixed bug XXXXX on 10/9/2012

vous pourriez avoir un commentaire qui dit

// doing X, because otherwise Y will break.

ou

// doing X, because doing Y is 10 times slower.
Dima
la source
12
+1 pour l'odeur de code des commentaires expliquant "quoi". Il est agréable de voir que les commentaires de code ne constituent pas un avantage automatique dans le sens où plus de commentaires> moins de commentaires. Je pourrais même revenir en arrière et penser qu'il y a des cas où même les commentaires décrivant "pourquoi" pourraient être une odeur indiquant que le code n'est pas clair. Par exemple, si je peux injecter un BubbleSorter ou un QuickSorter, le commentaire "J'utilise QuickSorter parce que c'est plus rapide" est superflu de la même manière que "injecter un quicksorter" est superflu. YMMV.
Erik Dietrich
53

Utilisez le meilleur outil pour le travail. Votre système de contrôle de version devrait être le meilleur outil pour enregistrer les corrections de bogues et les CR: il enregistre automatiquement la date et le nom de l'auteur du changement; il n'oublie jamais d'ajouter un message (si vous l'avez configuré pour exiger des messages de validation); il n'annule jamais la mauvaise ligne de code ni ne supprime accidentellement un commentaire. Et si votre système de contrôle de version fait déjà un meilleur travail que vos commentaires, il est ridicule de dupliquer le travail en ajoutant des commentaires.

La lisibilité du code source est primordiale. Une base de code encombrée de commentaires donnant l'historique complet de chaque correction de bogue et CR créés ne sera pas du tout lisible.

Mais n'omettez pas complètement les commentaires: de bons commentaires (ne pas documenter servilement chaque démarrage / arrêt / description / solution de chaque correction de bug ou CR) améliorent la lisibilité du code. Par exemple, pour un élément de code complexe ou peu clair que vous ajoutez à la résolution d'un bogue, un commentaire du formulaire // fix ISSUE#413indiquant aux internautes où trouver plus d'informations dans votre outil de suivi des problèmes est une excellente idée.

Josh Kelley
la source
29
Je suis d'accord sauf pour une chose: ce fix ISSUE#413n'est pas un bon commentaire dans le code. Vous devriez être capable de comprendre le code sans avoir à vous référer à la documentation externe. Au lieu de donner un nombre aléatoire, expliquez réellement pourquoi cette partie délicate du code est nécessaire pour faire quoi. C’est à cela que servent les commentaires: Expliquer les parties du code qui ne sont pas évidentes.
poke
12
@poke - Merci de l'avoir signalé. Je suppose que je devrais ajouter que le seul endroit où j'utilise les commentaires du formulaire fix ISSUE#413est l'endroit où le problème est si compliqué (un cas extrême extrêmement dépendant du système d'exploitation et de la configuration, ou uniquement déclenché par des données client erronées particulières) qui serait décrit correctement. quelques paragraphes; Ce genre de chose est mieux géré par un traqueur de problème, IMO. Même alors, une brève description est bonne.
Josh Kelley
8
@poke: Je dirais qu'un commentaire qui commence par fix ISSUE#413 est tout à fait correct, et même préférable, à condition qu'il fournisse également une quantité raisonnable d'informations sur la nature de la question 413. Résumer le rapport de problème sans fournir d'indicateur rend la vie plus difficile pour un futur lecteur qui a besoin de tous les détails.
Keith Thompson
Je suis d'accord avec poke - vous ne devriez jamais avoir à faire référence à une source externe pour comprendre le code. Si j'examine un changement, cela interrompt le processus. Je dois aller dans l'outil de suivi des problèmes, relever le problème et tout lire à ce sujet. Et que se passe-t-il si vous changez de suivi des problèmes? Il serait peut-être bon d’avoir fix ISSUE#413un commentaire complet, mais ne l’utilisez pas comme une béquille.
Michael Dean
"il n’oublie jamais d’ajouter un message (si vous l’avez configuré pour exiger des messages de validation); il n’annule jamais la mauvaise ligne de code ni ne supprime accidentellement un commentaire." Nous venons tout juste de gérer le fichier SVN se corrompant et devant restaurer à partir d'une sauvegarde. Nous avons pu trouver du code qui n'avait pas encore été sauvegardé, mais lorsque nous avons réenregistré les modifications, plusieurs commits distincts ne sont plus qu'un. Mon point n'est jamais un mot trop fort, et n'oublions pas les gens ne migrent pas vers un nouveau logiciel VCS, et l'historique de révision pourrait ne pas être réalisable ou possible.
Andy
7

Les commentaires dans le code concernent ce que le code est à ce moment. Prendre un instantané à un moment donné ne devrait pas faire référence aux anciennes versions du code (ou au pire, futures).

Les commentaires dans VCS concernent la modification du code. Ils devraient lire comme une histoire sur le développement.

Maintenant, chaque changement devrait inclure des commentaires? dans la plupart des cas, oui. La seule exception que j'imagine est lorsque le comportement attendu a déjà été documenté mais qu'il ne l'était pas à cause d'un bogue. En le corrigeant, les commentaires existants sont plus précis, ils ne doivent donc pas être modifiés. Le bogue lui-même devrait être documenté dans l'historique du ticket et dans le commentaire de validation, mais uniquement dans le code si le code semble étrange. Dans ce cas, un // make sure <bad thing> doesn't happendevrait suffire.

Javier
la source
8
Je voterais contre cela, mais je ne peux vraiment pas être d'accord avec "chaque changement doit inclure des commentaires? Dans la plupart des cas, oui.". Un commentaire check-in / commit, oui, absolument. Commentaires de code, certainement pas nécessairement.
un CVn
6

Un type de commentaire que j’apprécie vraiment est:

// Ceci est appliqué pour la règle métier 5 de la proposition 2

ou quel que soit le diable que vous utilisez pour répondre à vos besoins.

Cela présente deux avantages, l'un étant de vous permettre de trouver la raison pour laquelle vous avez implémenté un algorithme donné sans chercher, et d'autre part, de vous aider à communiquer avec des ingénieurs non-logiciels qui travaillent sur / créent les docs d'exigences.

Cela n’aidera peut-être pas les équipes plus petites, mais si vous avez des analyseurs qui développent vos exigences, cela peut être inestimable.

Bill K
la source
2
Cependant, c'est différent parce que cela fournit une traçabilité orthogonale au contrôle de version: la connexion entre le code et la spécification des exigences implémentée.
Kaz
Dans un système où le contrôle de version est associé au système de bogues / exigences, une traçabilité complète est fournie sans commentaire. Il est parfois utile de travailler dans l'autre sens. Avec un fichier de SCM, montrez-moi quelles exigences ont été implémentées quand. Ou, étant donné une exigence, affichez-moi tous les fichiers modifiés pour l'implémenter.
iivel
4

Vos interlocuteurs ont raison quand ils disent que les commentaires sont une bonne pratique de programmation, mais il existe des exceptions. L'ajout d'un commentaire pour chaque modification que vous apportez en fait partie. Et vous avez raison de dire que cela devrait appartenir au système de contrôle de version. Si vous devez conserver ces commentaires au même endroit, le VCS est la solution. Les commentaires dans le code source ont tendance à vieillir et à ne plus être entretenus. Aucun commentaire ne vaut mieux qu'un mauvais commentaire. Ce que vous ne voulez pas, c'est d'avoir des commentaires désynchronisés aux deux endroits (dans le code et le VCS). L'objectif est de garder les choses au sec en disposant d'une seule source de vérité pour les modifications du code.

marco-fiset
la source
3

Outre ce que d'autres ont dit, réfléchissez à ce qui se passe si un changement a des répercussions sur tout le système. Supposons que vous refactoriez une partie d'une interface principale dans le processus d'implémentation d'une demande de modification - ce type de modification peut facilement toucher un grand pourcentage des fichiers de code source de tout logiciel non trivial avec des modifications triviales (classe ou changement de nom de méthode). Êtes-vous censé parcourir chaque fichier touché par une telle opération pour l'annoter manuellement avec de tels commentaires, plutôt que de compter sur le VCS pour le faire automatiquement? Dans un cas, vous envisagez un travail d'un peu plus de cinq minutes avec n'importe quel outil de refactoring décent suivi d'un recompilation pour vous assurer que rien ne casse la construction, tandis que l'autre peut facilement se transformer en travail d'une journée. Pour quel bénéfice spécifique?

Pensez également à ce qui se passe lorsque vous déplacez des parties du code. L'un des développeurs de base de données avec laquelle je travaille est dans le camp de "en grande partie" chaque ligne de SQL doit être annotée avec la révision dans laquelle elle a été modifiée, et nous allons faire des historiques de révision séparés pour chaque fichier, car il est alors plus facile de voir qui a changé quoi quand et pourquoi ". Cela fonctionne assez bien quand le changement estdans l'ordre de changement de lignes simples. Cela ne fonctionne pas aussi bien lorsque, comme je l'ai fait récemment pour résoudre un problème de performances grave, vous séparez des parties d'une requête plus large introduisant des tables temporaires, puis modifiez l'ordre de certaines des requêtes pour mieux correspondre au nouveau flux de code. Certes, le diff par rapport à la version précédente était en grande partie dépourvu de sens, car il était indiqué que les deux tiers environ du fichier avaient été modifiés, mais le commentaire d’enregistrement ressemblait aussi à une "réorganisation majeure pour résoudre les problèmes de performances". Au moment où vous avez examiné manuellement les deux versions, il était assez clair que les grandes pièces étaient vraiment les mêmes, seulement déplacées. (Et il a fallu à la procédure stockée en question une prise de temps régulière pour prendre une demi-minute, à quelques secondes. À ce moment-là,

À de très rares exceptions près, le suivi des modifications et le référencement des problèmes sont le travail du VCS, IMNSHO.

un CVn
la source
3

Je respecte généralement cette règle: si le changement est évident et que le code résultant ne soulève pas de questions, ne rétablit pas ou ne modifie pas substantiellement tout comportement précédent de manière substantielle - puis laissez le VCS pour suivre les numéros de bogues et autres informations de changement.

Cependant, s’il ya un changement qui n’est pas évident, cela change la logique - en particulier, modifie substantiellement la logique effectuée par quelqu'un d’autre part - il peut être très bénéfique d’ajouter quelque chose comme "ce changement est de le faire et cela à cause du bogue n ° 42742 ". De cette façon, quand quelqu'un regarde le code et se demande "pourquoi est-ce ici? Ça a l'air bizarre", il est guidé juste devant lui et n'a pas à mener une enquête via VCS. Cela évite également les situations dans lesquelles les utilisateurs annulent les modifications des autres parce qu'ils sont familiarisés avec l'ancien code mais ne remarquent pas qu'il a été modifié depuis.

StasM
la source
2

Les commentaires liés au contrôle de version n'appartiennent pas au fichier source. Ils ne font qu'ajouter de l'encombrement. Comme ils doivent probablement être placés au même endroit (comme un bloc de commentaires en haut du fichier), ils provoqueront des conflits de nuisance liés aux commentaires uniquement lorsque des branches parallèles sont fusionnées.

Toute information de suivi pouvant être extraite du contrôle de version ne doit pas être dupliquée dans le corps du code. Cela vaut pour des idées idiotes telles que les mots clés de la commande RCS, par exemple $Log$.

Si le code dépasse le cadre du système de contrôle de version, cette suite de commentaires sur son historique perd son contexte et par conséquent l'essentiel de sa valeur. Pour bien comprendre la description de la modification, nous avons besoin d'accéder à la révision afin de pouvoir afficher le diff par rapport à la version précédente.

Certains anciens fichiers du noyau Linux ont de gros blocs de commentaires d'historique. Celles-ci remontent à l'époque où il n'y avait pas de système de contrôle de version, juste des archives et des correctifs.

Kaz
la source
2

Les commentaires dans le code doivent être minimes et précis. Ajouter des informations sur les défauts / modifications n'est pas utile. Vous devriez utiliser le contrôle de version pour cela. Quelques temps Le contrôle des versions fournit un moyen légèrement meilleur de changement - Nous utilisons ClearCase UCM; Les activités UCM sont créées en fonction des numéros de défaut, de la zone de modification, etc. (par exemple, defect29844_change_sql_to_handle_null).

Les commentaires détaillés sont préférés dans les commentaires d'enregistrement.

Je préfère inclure les détails sur les informations de base, les détails de la solution NON mis en œuvre en raison de certains effets secondaires.

Pramagic Programmer et CleanCode mènent à la directive suivante

Conservez les connaissances de bas niveau dans le code, là où elles appartiennent, et réservez les commentaires pour d'autres explications de haut niveau.

Jayan
la source