Discussion :

[LinuxUser]

Bonjour,

Je voulais connaitre un peu pour vous les bonnes pratiques et conventions lorqu'on commente son code C++, ce qui pour vous est absolument à faire et à éviter.

J'aimerai aussi avoir votre avis sur deux choses:

1/ Doit-on absolument séparer prototype (headers) et implémentation (.cpp), ou peut implémenter par exemple les constructeurs et getters/setters dans le .h pour "alléger" le .cpp?
2/ Comment bien commenter un getter ou setter sans mettre un commentaire inutile du genre:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
// return the string length
int getLength();

Merci de votre aide.

[Neckara]

Bonjour,

Pour commenter j'utilise des commentaire doxygen :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
/** @brief Description
   @param int x : parametre
   @return bool : valeur de retour */

Avec @enum pour les énumérations, @mainpage pour la description du programme, etc...

Après, on ne devrait pas à avoir à mettre des commentaires dans le code (sauf cas exceptionnel) car ceci signifierais que le code n'est pas assez explicite.

Si tu veux "alléger" le .cpp c'est peut être que ta classe a trop de responsabilité, et il vaut mieux dans ce cas la faire plusieurs classes.

On ne met l'implémentation dans les .h uniquement quand on veut inliner des méthodes ou fonctions ou quand on utilise des templates.

[LinuxUser]

Intéressant, tu dis que le code en soit necessite peu voire pas de commentaire, j'ai toujours cru au contraire qu'il fallait "tout" commenter.

Mis à part les inline et template, pas de code (constructeurs par exemple) dans les .h si je comprends bien.

Par contre, pour en revenir aux commentaires, doit-on commenter une fonctions deux fois (une fois son prototype dans le .h et une seconde fois dans .cpp) ou juste dans le .h suffit?

.h

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
 
/**
* @desc calcul force correction with rolling mill's coefficients
* @return double - force corrected
*/
double forceCorrection();

.cpp

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
 
// on répète le commentaire du .h ou pas?
double forceCorrection()
{
...
}

[Neckara]

Intéressant, tu dis que le code en soit necessite peu voire pas de commentaire, j'ai toujours cru au contraire qu'il fallait "tout" commenter.

Trop de commentaires tuent le commentaire :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
void foo(void)//fonction foo
{
       int i; //déclaration d'un entier nommé i
       if(true) //si vrai
             i = 7; // i vaut alors 7
}

Normalement sauf cas exceptionnels, un code bien indenté avec des noms de variables et de fonction explicites devraient suffire à faire comprendre ce que fait le code sans que des commentaires dans le code soit vraiment utiles (sauf commentaires style TODO etc...).
Par contre les commentaires doxygen permettent de générer une documentation qui permet à l'utilisateur de ne pas à avoir à fouiller dans le code pour pouvoir l'utiliser.

Par contre, pour en revenir aux commentaires, doit-on commenter une fonctions deux fois (une fois son prototype dans le .h et une seconde fois dans .cpp) ou juste dans le .h suffit?

Une seule fois suffit mais certaines personnes préfèrent les mettre dans le .cpp pour rendre la compilation plus rapide.

[kinaesthesia]

Pour ma part on ne commente que le header car c'est bel et bien ce fichier qui définit une classe.

Un autre programmeur va utiliser ta classe, il va simplement regarder le header pour connaître l'objet et non l'implémentation qu'il y a derrière. C'est de l'encapsulation.

Quand tu utilises une librairies, tu ne regardes pas le code source (sauf cas exceptionnel) ?

Les commentaires dans l'implémentation doivent être exceptionnel du genre :

// TODO
// FIXME
// Could be better

...

Mais uniquement pour toi /ou un autre dev qui devrait changer ton implémentation.


Pour les getters, je ne met point de code sauf s'il y a un quelconque traitement dedans.

[LinuxUser]

Pour ma part on ne commente que le header car c'est bel et bien ce fichier qui définit une classe.

Je suis d'accord, je pense la même chose.

Pour les getters, je ne met point de code sauf s'il y a un quelconque traitement dedans.

Par contre, pour moi, un getter() doit uniquement renvoyer une valeur sans aucun traitement (conversion, mise à l"chelle, ou autre), sinon ce n'est plus un getter() mais une fonction en soit et son appelation de getter devient trompeuse.

En gros, un getter devrait (ou doit si on est stricte) ressembler à cela:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
// je sais pas si un commentaire est neccesaire ici, d'après kinaesthesia, non
double getValue()
{
  return m_value;
}

[Neckara]

Pour ma part on ne commente que le header car c'est bel et bien ce fichier qui définit une classe.

Un autre programmeur va utiliser ta classe, il va simplement regarder le header pour connaître l'objet et non l'implémentation qu'il y a derrière. C'est de l'encapsulation.

Personnellement, je pense que le programmeur ne devrait même pas regarder le header, la documentation (si possible doxygen) devant normalement lui suffire.

Même si je n'en suis pas très fan, le fait de mettre les commentaires dans le .cpp peut parfois être un gain de temps non-négligeable dans de très gros projets, en effet dès qu'on va modifier un commentaire dans un header, on va devoir recompiller tous les .cpp incluant ce header.

Pour les getters, je ne met point de code sauf s'il y a un quelconque traitement dedans.

Personnellement je les commente toujours, si possible j'essaye d'ajouter des précisions/informations sur la variable retournée.

Par contre une chose à ne jamais faire :
Ne mettre presque aucun commentaire sauf quelques jeux de mots avec getter et setter. ça ne fait pas tellement rire quand on reprend le code par la suite.

Par contre, pour moi, un getter() doit uniquement renvoyer une valeur sans aucun traitement (conversion, mise à l"chelle, ou autre), sinon ce n'est plus un getter() mais une fonction en soit et son appelation de getter devient trompeuse.

Je ne suis pas du tout d'accord, en quoi l'appellation serait-elle trompeuse?
L'utilisateur n'est pas censé savoir que la variable existe tel quel ou non.

[kinaesthesia]

Personnellement, je pense que le programmeur ne devrait même pas regarder le header, la documentation (si possible doxygen) devant normalement lui suffire.

Même si je n'en suis pas très fan, le fait de mettre les commentaires dans le .cpp peut parfois être un gain de temps non-négligeable dans de très gros projets, en effet dès qu'on va modifier un commentaire dans un header, on va devoir re-compiler tous les .cpp incluant ce header.

Pourquoi tu voudrais modifier un commentaire ? Car la fonction a changé ? tu peux très bien gérer ça du coté de ton makefile. Puis on ne change pas le commentaire d'une fonction subitement comme ça.

[Neckara]

Pourquoi tu voudrais modifier un commentaire ? Car la fonction a changé ? tu peux très bien gérer ça du coté de ton makefile. Puis on ne change pas le commentaire d'une fonction subitement comme ça.

Parce qu'on a fait une erreur, une faute d'orthographe, qu'on s'est aperçut que le commentaire n'est pas assez clair, qu'on veut rajouter un exemple, qu'on a modifié la cuisine interne de la classe voir même qu'on ai l'habitude de commenter correctement la classe une fois stable/finie.

Sinon je vois mal comment tu gère cela au niveau du makefile.

[LinuxUser]

Je ne suis pas du tout d'accord, en quoi l'appellation serait-elle trompeuse?
L'utilisateur n'est pas censé savoir que la variable existe tel quel ou non.

C'est seulement mon avis, un getter ne devrait que retourner une simple valeur sans traitement ni modification car pour moi c'est une opération ultra basique (voire atomique).
Si on a un besoin de traitement sur un membre, on fait une fonction pour cela, même si c'est un tout petit traitement.

Et parfois, appeller une fonction getSomething() alors qu'elle fait plus que juste renvoyer la valeur peut être trompeur, j'en ai fait récemment l'expérience, exemple:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
class Foo
{
private:
  double m_force;
public:
  double getForce();
};

Pour moi getForce() doit ressembler à cela:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
double getForce()
{
  return m_force;
}

Or la force retournée était bien celle que j'attendais, sauf lorsque j'affichais sa valuer lors de calcul intermédiaire, pour la simple et bonne raison qu'elle était converti (en Newton) à l'initialisation et lors du getter():

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
Foo::Foo(double force)
{
  m_force = force*9.81; 
}
double getForce()
{
  return m_force/9.81;
}

Donc si dans une focntion de Foo j'affiche la valeur de m_force elle sera "fausse" car je m'attends à avoir une valeur en Kg et non en Newton, surtout si aucun commentaire ne prévient.

Alors tu me diras qu'il suffit de modifier en:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
class Foo
{
private:
  double m_forceNewton;
public:
  double getForceKg();
};

Et que tout ètait de la faute du développeur qui n'a rien commenter (et je suis d'accord), mais je suis pas trop fan de ce genre de choses.

[Neckara]

Si tes getter et tes setter ne font que :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
void set(int i)
{
       m_i = i;
}
 
int get(void)
{
      return m_i;
}

et dès qu'elles font plus de choses, tu changes le nom de la méthode, alors les getter et setter sont inutiles, autant mettre la variable en public.

[LinuxUser]

Ca me trouble beaucoup ce que tu me dis là, car je m'efforce à ce que mes getters et setters ressemblent à ton exemple, pensant bien faire en faisant cela .

[Neckara]

Ca me trouble beaucoup ce que tu me dis là, car je m'efforce à ce que mes getters et setters ressemblent à ton exemple, pensant bien faire en faisant cela .

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
void set(int i)
{
       m_i = i;
}
 
int get(void)
{
      return m_i;
}

Ne sert à rien et il vaut mieux mettre la variable en public.
Si on met quand même un setter et un getter au lieu de mettre la variable en public, c'est pour rendre le code maintenable, en effet si un jour on souhaite rajouter des traitements lors du getter/setter, on aura juste la "cuisine interne" à modifier sans avoir à modifier le reste du code (ie tous les appels aux getter et setter).

[Klaim]

Si tes getter et tes setter ne font que :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
void set(int i)
{
       m_i = i;
}
 
int get(void)
{
      return m_i;
}

et dès qu'elles font plus de choses, tu changes le nom de la méthode, alors les getter et setter sont inutiles, autant mettre la variable en public.

Ce n'est applicable que si ton object represente des donnees brutes sans possible verification. Autrement dis c'est assez rare.
Si dans le future tu veux ajouter des traitements ou verifications alors tu seras bien ennuye d'avoir a changer tout le code qui utilise ce type pour ajouter des paranetheres pour acceter aux fonctions au lieu des membres.

[kinaesthesia]

Ca me trouble beaucoup ce que tu me dis là, car je m'efforce à ce que mes getters et setters ressemblent à ton exemple, pensant bien faire en faisant cela .

Mais tu remarqueras que cela n'a pas de sens puisqu'on peut depuis l'extérieur modifier la variable sans traitement/vérification.

Comme dit Neckara, si tes getter et setter ne font que ça, met ta variable en public ^^

[Neckara]

Je pense que j'ai été mal compris

Les getters/setters du type :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
void set(int i)
{
       m_i = i;
}
 
int get(void)
{
       return m_i;
}

N'ont d'utilité que pour la maintenabilité du code où on peut facilement modifier les traitements/vérifications sans modifier tous les appels aux getter/setter dans le code.

Mais si dès qu'on rajoute des traitements/vérification, on renomme les méthodes (ie au lieu de mettre getMachin() on met machin() ) on perd cette utilité.

De là mettre la variable en public revient exactement au même sauf qu'on gagne deux appels de méthodes (sauf si inline).

Mais il vaut mieux utiliser des getter/setter plutôt que de mettre un attribut en public justement pour des raisons de maintenabilité à condition bien sûr de ne pas renommer les getter/setter à chaque modification.
Pour ne pas perdre de performances, on peut inliner ces getter/setter.

[Bousk]

C'est seulement mon avis, un getter ne devrait que retourner une simple valeur sans traitement ni modification car pour moi c'est une opération ultra basique (voire atomique).
Si on a un besoin de traitement sur un membre, on fait une fonction pour cela, même si c'est un tout petit traitement.

Et parfois, appeller une fonction getSomething() alors qu'elle fait plus que juste renvoyer la valeur peut être trompeur, j'en ai fait récemment l'expérience, exemple:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
class Foo
{
private:
  double m_force;
public:
  double getForce();
};

Pour moi getForce() doit ressembler à cela:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
double getForce()
{
  return m_force;
}

Or la force retournée était bien celle que j'attendais, sauf lorsque j'affichais sa valuer lors de calcul intermédiaire, pour la simple et bonne raison qu'elle était converti (en Newton) à l'initialisation et lors du getter():

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
Foo::Foo(double force)
{
  m_force = force*9.81; 
}
double getForce()
{
  return m_force/9.81;
}

Donc si dans une focntion de Foo j'affiche la valeur de m_force elle sera "fausse" car je m'attends à avoir une valeur en Kg et non en Newton, surtout si aucun commentaire ne prévient.

Alors tu me diras qu'il suffit de modifier en:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
class Foo
{
private:
  double m_forceNewton;
public:
  double getForceKg();
};

Et que tout ètait de la faute du développeur qui n'a rien commenter (et je suis d'accord), mais je suis pas trop fan de ce genre de choses.

Ceci est la responsabilité de la classe, si la classe a dit que la force était en Newton, c'est toi qui est en tord.
Si rien n'est précisé, aucune conclusion n'est possible, ni même la supposition qu'il s'agit de Kg.

[koala01]

Salut,

Pour ce qui est des commentaires, je ferai la distinction entre les commentaires qui permettent la génération de documentation automatique et les autre

Pour ce qui est des commentaires de génération automatique de documentation ("doxygen", par exemple ), je trouve qu'il doivent etre complets et cohérents :

• indiquer l'objectif poursuivi par la fonction
• indiquer à quoi servent les arguments
• indiquer clairement la valeur de retour s'il y en a
• indiquer explicitement les pré / post conditions /invariants
• indiquer les exceptions que la fonction elle-même (je ne parle pas forcément des fonctions appelées) est susceptible de lancer
• Eventuellement, donner un code d'exemple

Pour les autres commentaires

A priori, le code ne devrait pas en avoir besoin et devrait se suffire à lui même, mais, si commentaire il y a, il devraient se limiter à décrire le but poursuivi par la partie du code à laquelle ils se rapportent, plutôt que d'expliquer ce que fait le code

En effet, si l'algorithme est à ce point compliqué qu'il faut en arriver à le commenter, le fait que le commentaire indique ce qui est espéré rendra les commentaires beaucoup plus stables et permettront, le cas échéant, de se rendre compte que le code ne fait pas ce qu'il devrait (et donc, rendra la correction sans doute plus facile).

Enfin, concernant le débat sur les accesseurs et mutateurs.

J'avouerai ne pas avoir tout lu, mais:
A priori, la loi demeter nous inciterait à éviter les accesseurs pour la simple raison qu'ils nous obligent bien souvent à connaitre "ce que la classe utilise en interne" pour pouvoir travailler avec elle.

En ce qui concerne les mutateurs, c'est encore pire : Bien souvent, la mise à disposition d'un mutateurs se traduit, au final, par un code proche de

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
Truc var = obj.getTruc();
/* modification de var */
obj.setTruc(var);

ce qui est non seulement purement horrible en regard de la loi demeter, mais qui, en plus, risque d'occasionner des répétitions en masses et des appels en cascades aux accesseurs et aux mutateurs des objet ainsi récupérés.

Il est alors beaucoup plus simple, efficace et "error prone" de fournir directement un comportement dont le nom est adapté, prenant des données "simples" (on va dire : des types primitifs et des chaines de caractères éventuellement, même si cela peut parraitre excessif ) comme arguments qui s'occuperont d'apporter les modifications adéquates.

Cela permettrait, en outre, d'éviter l'impression "d'immédiateté" des mutateurs en laissant penser qu'il peut s'agir d'une action "prenant un temps relativement long".

Par exemple, si l'on a une classe qui se positionne grace à un point 2 dimensions, au lieu d'avoir un code proche de

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
Point recup = obj.getPoint();
recup.setX(recup.getX() + 2);
recup.setY(recup.getX() - 5);
obj.setPoint(recup);

le code ressemblerait à quelque chose comme

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
/* soit move(distanceX, distanceY) */
move(2,-5); 
/* avec le comportement de move pouvant être de l'ordre de
move(distanceX, distanceY)
{
    m_position = Point(m_positon.x() + distanceX,
                           m_position.y() + distanceY);
}
car Point est, classiquement, une classe ayant sémantique de valeur ;)
*/

Cela pemettrait à l'utilisateur de n'avoir à s'intéresser à ce qui représente la position (comprenez : au point selon mon exemple ) que s'il a réellement besoin de manipuler un point, ce qui est, en définitive, relativement rare

[r0d]

Concernant les commentaires dans le code (dans le .cpp), je trouve que les développeurs n'en mettent pas assez. C'est un avis personnel bien sûr.
Je préfère, en effet, écrire les "notes de developpement" dans le code plutôt que dans l'en-tête, car je préfère que l'en-tête et la doc générée automatiquement n'entre pas dans les détails d'implémentation.
Par exemple, un fonction implémente un algorithme connu mais y ajoute une légère variation selon les besoins du contexte. Cette modification aura, AMHA, bien souvent sa place dans le code source, car elle ne concernera que la personne qui devra faire des modifications.
Les petites astuces d'implémentation également doivent être, AMHA, commentées dans l'implémentation elle-même. etc.

[EtherOS]

Je suis du même avis . j' ajouterais la génération des commentaires avec :

=======SOFT POUR GEN COMMENT ==============

=> DoxyBlocks dans le nouveau CodeBlocks 12.11 pour générer des commentaires aux formats(*.RTF,*.HTML, *.CHM,etc...)

=> MiKTeX pour générer des commentaires sous format pdf

=> Doxygen (sudo apt-get install doxywizard pour installer doxygen sous linux)

=======METHODOLOGIE DE REDACTION D'UN COMMENT======

deja le comment(doxygen) est de la forme :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
/** @def __MAX  valeur maximale */
#define __MAX  SIZE


/**
 * @fn float ** merci(int*, double**,char*,int)
 * @param   \b param1 
 * @param   \b param2
 * @param   \b param3
 * @param   \b param4
 * @brief   votre briefing concernant la fonction
 * @details  les détails de la fonction
 * @return  ce qu'elle retourne
 */

// \b met en gras le texte qui le suit
float** merci(int* param1,double** param2,char* param3,int param4);
{
  /* Ecrire votre code ici*/

   // Boe Année 2013

  /*ou encore */

 /** ce commentaire est pris en charge par doxygen */
}

Concernant les getter , setter je suis d'avis avec Bousk