Accueil > .Net, C#, Humeur > Commenter son code

Commenter son code

Les commentaires, c’est comme tout.
Y a les bons commentaires et les mauvais commentaires.
Un bon commentaire, c’est un commentaire sur le code, mais c’est un bon commentaire.
Un mauvais commentaire, c’est un commentaire sur le code, mais c’est un mauvais commentaire.

Bon, outre cette introduction en clin d’oeil à des humoristes comme on en fait plus, il y a le réel problème des commentaires dans le code.
Pourquoi réel ?
Parce que certains commentaires brillent par leurs absences, certains sont tellement obscurs qu’ils n’éclairent en rien, les commentaires qui sont vides, et enfin les commentaires qui sont carrément faux, résidu de commentaire qui a été pertinent à une époque plus ou moins déterminée.

Un commentaire que j’avais rencontré :

Puisque le chef de projets est une vraie buse, il a totalement sous-chiffré le projet. Donc, j’ai pas le temps de faire mieux que le code présent. Une fois en production, ça ne sera pas à moi de le maintenir, donc : bon courage. Et quand quelqu’un lira ce commentaire, je serais déjà parti !

Je n’ai pas reproduit fidèlement, « une vraie buse » n’étant pas le texte originel…mais bon…🙂


 

Voici quelques exemples :

/// <summary>
/// 
/// </summary>
/// <param name="unparametre"></param>
/// <param name="etunsecond"></param>
public String MaMethode(String unparametre, Int32 etunsecond)
{
    // Correction anomalie, JP, 10/08/2009
    if (String.IsNullOrEmpty(unparametre)
        && etunsecond != Int32.MinValue)
    {
        // Concaténation des cinq paramètres pour former le message
        return Properties.Messages.ResourceManager.GetString(String.Concat(unparametre, etunsecond));
        //TODO: modifier la gestion des messages pour la rendre plus flexible
    }
    else
        return String.Empty;
}

Clairement, le commentaire sur la méthode est présent pour éviter un warning lors de la compilation.

Le commentaire sur la correction ne sert à rien puisqu’il ne dit pas sur quoi porte la correction, on apprend juste que la personne l’ayant faite à pour initiales JP, qu’elle a été faite le 10/08/2009 et que le gars est sans doute partit depuis. Utile, y a pas à dire…

L’avant dernier ne correspond pas à ce qui est fait (cinq paramètres) et donc peut induire en erreur : C’est ce qui ETAIT fait ? Dans ce cas, quand est-ce que ça a été modifié et pourquoi ? C’est ce ce qui DOIT être fait ?

Enfin, mon préféré, le fameux « TODO ». Si un TODO n’est pas traité dans les 2 jours (voir même dans la journée), il ne le sera jamais.

Il y a bien sûr tout un tas d’exemples potentiels, mais je pense que vous comprenez l’idée.

Alors, c’est quoi un bon commentaire ???
On va passer sur ce qui existe, ce qu’il est possible de faire et un petit avis personnel (issu que ce que je trouve pratique, maintenable et de l’expérience).

 

Les cartouches.

 
En général, on retrouve les cartouches en début de fichier, avec plein de *, un formatage complexe et tout plein d’infos inutiles (qui a créé le fichier, quand, quel est son nom…).

Le meilleur cartouche est celui…qui n’existe pas.
Le contrôle de code source porte ses informations, elles sont donc parfaitement inutiles et de toute manière rarement mise à jour….

Le cartouche est donc utile exclusivement dans le cas où : il porte des informations simples, il est mis à jour et surtout, quand le fichier n’est pas soumit au contrôle de code source (ce qui peut arriver avec des scripts de base de données, même si le fait qu’ils soient en dehors du contrôleur est discutable).

 

Le code doit se commenter lui-même.

 
Qui n’a jamais codé un truc et devoir le reprendre plusieurs mois/années plus tard ?
Et oui, ça fait mal. Souvent, on pense « mais quel abruti à coder ça ??? ». La réponse correcte étant souvent la plus simple : celui qui demande.

En conséquence, le code doit se commenter lui-même, ce qui veut dire d’avoir des noms de variables explicites, du code aéré et lisible (mettre un espace d’une ligne n’a jamais tué personne), c’est un début.
Souvent, une variable nommée correctement évitera le besoin d’un commentaire.
Ceci dit, ce n’est pas non plus la peine d’avoir des noms de variables de 50 caractères…

Dans les scopes très réduits, il est possible d’avoir des noms très courts (le fameux i dans la boucle for, même si la nommer index ne fait pas de mal), mais sinon, il vaut mieux éviter.

Après, penser à la largeur d’écran.
Les écrans sont de plus en plus larges, avec des résolutions de plus en plus élevées, mais…plus souvent chez soi qu’au travail.
Passé 80-100 caractères, il peut être bon de faire un retour à la ligne pour gagner en lisibilité.
Dans ce cas, il peut être pertinent de mettre un retour à la ligne pour chaque paramètre d’une méthode, par exemple, avec l’indentation qui va bien. Ou quand on appelle des méthodes en cascade (les requêtes complexes sur Linq en sont un bon exemple).

        // Exemple pas trop lisible
        public void TraitementGeneral()
        {
            DonneesTraitement donneesTraitement = new DonneesTraitement() { IdentifiantTraitement = "TraitementCourant", DebutTraitement = DateTime.Now };
            try
            {
            }
            catch (BusinessRuleException bre)
            {
                Toolbox.InsertionMessage(TypeMessage.ERR, donneesTraitement.IdentifiantTraitement, donneesTraitement.DebutTraitement, Toolbox.FormatageException(bre));
            }
        }
        // Exemple lisible
        public void TraitementGeneral()
        {
            DonneesTraitement donneesTraitement = 
                new DonneesTraitement() { 
                    IdentifiantTraitement = "TraitementCourant" , 
                    DebutTraitement = DateTime.Now 
                };
            try
            {
            }
            catch (BusinessRuleException bre)
            {
                Toolbox.InsertionMessage(
                    TypeMessage.ERR, 
                    donneesTraitement.IdentifiantTraitement, 
                    donneesTraitement.DebutTraitement, 
                    Toolbox.FormatageException(bre));
            }
        }

C’est exemple est encore plus parlant quand on l’affiche au sein d’un blog, comme ici, avec des largeurs de colonnes fixes.
Qui n’a jamais vu, sur un blog, une ligne de code qui disparaît sous le bloc d’à coté ???

 

Les régions.

 
Les régions, c’est le bien (couple #region et #endregion)
Mais c’est comme tout, à consommer avec modération (qui que soit modération !).

Les régions sont, à mon sens, faites pour séparer les différentes fonctionnalités par bloc logiques.
Par exemple, j’aime bien mettre des régions autour des propriétés, des constructeurs, membres d’interfaces/classes, etc.

Mais…dans le cas du Single responsibility principle, il faut se demander si les dites régions ne regroupent pas des fonctionnalités qui devraient aller voir ailleurs.

Quoi qu’il en soit, il est possible de mettre un descriptif bref sur les régions. Et il est même possible de mettre la même description sur le endregion.
Cela permet de voir quand ce finit un bloc. C’est parfois con, mais ça peut être utile.

 

Attribut

 
Il existe également un attribut portant le nom ô combien pertinent de DescriptionAttribute (dans System.ComponentModel).
Cet attribut prends en paramètre une chaîne de caractères.
Il est utilisé, en autres, par l’interface de l’explorateur de tests, avec la colonne nommée…description.
Et l’avantage, c’est cet attribut peut être placé sur n’importe quoi (étant décoré lui-même de AttributeUsageAttribute(AttributeTargets.All)).

 

Que mettre dans un commentaire ?

 
En général, en lisant le code, on doit pouvoir comprendre ce qui est fait (enfin, ça, c’est la théorie).
Le commentaire peut aider à comprendre le pourquoi c’est fait et pourquoi c’est fait comme ça.
Quand un commentaire nous indique le but du code, pourquoi il a été codé (dans quel cas l’utiliser) et surtout pourquoi il a été fait comme ça, c’est un gain de temps assez énorme.

Exemple simple, quand on doit reprendre un code (que l’on a pas produit) et qui a un bug, il est parfois utile d’avoir des commentaires qui expliquent ce qui est fait, histoire d’être aiguillé plus vite vers le bout de code qui pose problème (et qui parfois s’en trouve être légitimé et renvoie la balle à un trou dans les specs, mais c’est un autre sujet : « It’s not a bug, it’s a feature »).

 

Et le gestionnaire de code source ?

 
Le commentaire quand on archive…et bien, c’est aussi un commentaire.
Donc ce que j’ai marqué dans la section précédente s’applique.

Mais pour bien commenter un check-in, il est assez pertinent de faire des check-in plus régulier.
Autant en phase de développement d’une nouvelle application, ça peut aller, autant en phase de recette, il est pertinent de découper ses check-in par anomalie.
Et là (ou pif), TFS aide beaucoup en permettant de lier un check-in avec un defect. En l’absence de TFS, il est toujours possible de mettre…un commentaire pour dire quelle anomalie est corrigée.
Mais surtout, il faut éviter de faire un check-in par jour avec une cinquantaine de fichiers et un vrai changelog…ça devient parfois ridicule.

 

Les magic string

 
Cela n’entre pas réellement dans le cadre des commentaires, mais cela améliore grandement lisibilité, maintenabilité et lutte efficacement contre ce qui est nommé « l’odeur du code » (code smell).

Pour rappel, les magic string sont des variables de type string de ce genre :

private String GetLongVariable(String variable){
            
    switch (variable)
    {
        case "X": return "Soemthing";
        case "Y": return "Another thing";
        case "Z": return "A last thing";
        default: return "I don't know";
    }
}

Dans cet exemple, les variable de retour sont des magic string.
Ce qui veut dire qu’elles, entre autres, sont sensibles aux fautes de frappe (soemthing).
Ce qui veut dire aussi que si je fais une modification dans un coin, cela n’impacte pas forcément tous les éléments qui utilisent ces variables (ex: si je fais un bête ctrl+h pour modifier « Something » en « First of all », je ne prendrais pas ma faute de frappe).

Il y a deux cas pertinent : remplacer les variables par des énumérations (pas toujours faisable) ou par une classe statique qui contient des constantes.
Dans les deux cas, le changement de valeur ne se produit qu’à un seul et unique endroit.

 

Conclusion

 
Tout ça pour quoi ?
Parce que c’est pénible de reprendre un code qui n’est pas lisible.
C’est tout🙂
Il y a tellement de moyen de faire du code propre et lisible.
Et puis, une bonne documentation XML permet aussi de construire des docs style javadoc/msdn avec Sandcastle ou Doxygen.

Pour rire, il existe un sujet sur Stackoverflow qui liste pas mal de commentaires assez…What is the best comment in source code you have ever encountered?. Honnêtement, ça vaut le coup d’oeil et exemple parfaitement certaines choses que j’ai dis plus haut.

Mais surtout, une citation d’un de mes chefs de projets, valables pour les commentaires ainsi que pour beaucoup d’autres choses :
Code comme si le gars qui allait te remplacer était un dangereux psychopathe qui sait où tu habites.

Catégories :.Net, C#, Humeur
  1. 19/10/2012 à 21:33

    Très bon article ! Il est bon de répéter les bonnes pratiques concernant l’indentation et les commentaires … trop souvent négligés.

    Surtout concernant les commentaires qui précisent le trigramme de l’auteur et la date de réalisation. Sous TFS, on peut d’ores et déjà retrouver cette information dans les annotations, et potentiellement le Work Item associé.

    Une petite remarque toutefois concernant le code en exemple, il est conseillé d’utiliser l’objet « string » au lieu de la classe « String » directement, notamment pour l’appel de méthodes comme « IsNullOrWhiteSpace » (préférable à IsNullOrEmpty, mais seulement utilisable depuis la version 4 de .NET).

    • 19/10/2012 à 21:49

      Merci à toi🙂
      Pour les commentaires, c’est à force de reprendre du code, ça me hérisse un peu, par moment…d’où ce billet.

      Une source pour string à la place de String ?
      Je n’en ai pas trouvé qui fasse réellement la distinction.
      Maintenant, je mets toujours « String », pour la coloration syntaxique (c’est peut être con, mais ça m’aide à lire le code…).

      Pour IsNullOrWhiteSpace, tu as raison.
      Ceci dit, je travaille en permanence sur les Frameworks 2.0, 3.0, 3.5 et 4.0, alors parfois…

      Par curiosité, j’ai regardé comment c’était fait :

      public static bool IsNullOrEmpty(string value)
      {
      	return value == null || value.Length == 0;
      }
      public static bool IsNullOrWhiteSpace(string value)
      {
      	if (value == null)
      	{
      		return true;
      	}
      	for (int i = 0; i < value.Length; i++)
      	{
      		if (!char.IsWhiteSpace(value[i]))
      		{
      			return false;
      		}
      	}
      	return true;
      }
      
  2. 14/11/2012 à 16:09

    Alors je n’ai pas de source sûre, mais actuellement on utilise un logiciel qui analyse le code en fonction de « best practices » de codage C# .NET, et il recommandait d’utiliser string au lieu de String. Après c’est vrai que je n’ai pas d’autres sources à ce sujet😦

    • 16/11/2012 à 11:33

      Quel outil, par curiosité ?
      Comme je travaille encore pas mal sur des solutions assez anciennes (même si on fait du neuf dedans), un outil de ce genre, c’est un peu risqué ^^

      Mais bon, je reste quand même sur « String » pour la coloration syntaxique !

  1. No trackbacks yet.

Laisser un commentaire

Entrez vos coordonnées ci-dessous ou cliquez sur une icône pour vous connecter:

Logo WordPress.com

Vous commentez à l'aide de votre compte WordPress.com. Déconnexion / Changer )

Image Twitter

Vous commentez à l'aide de votre compte Twitter. Déconnexion / Changer )

Photo Facebook

Vous commentez à l'aide de votre compte Facebook. Déconnexion / Changer )

Photo Google+

Vous commentez à l'aide de votre compte Google+. Déconnexion / Changer )

Connexion à %s

%d blogueurs aiment cette page :