Discussion :

[Steph_ng8]

Bonjour,
J'imagine qu'il y a déjà (eu) un « débat » sur ce sujet, mais je ne sais pas exactement où chercher...

Bref, pour le cas où l'intitulé ne serait pas assez explicite, j'aimerais savoir quelle est la meilleure façon (selon vous) d'écrire une classe dont plusieurs constructeurs partagent un bout de code :

1. avec une classe-mère dont le seul but est de factoriser ce bout de code dans son propre constructeur ;
2. écrire une fonction spécifique pour ce bout ce code.

Au cas où je ne serais toujours pas clair, je me retrouve dans cette situation :

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
class uneClasse
{
 
    public:
        uneClasse(...) { // Un constructeur
             // Code partagé
             ...
 
             // Code spécifique
             ...
        }
 
        uneClasse(...) { // Un autre constructeur
             // Code partagé
             ...
 
             // Code spécifique
             ...
        }
 
        ...
 
};

Quelle la meilleure solution pour factoriser le code partagé ?

Cas 1 :

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
28
29
30
31
32
class _base
{
 
    protected:
        _base(/* paramètres éventuels */) {
            // Code partagé
            ...
        }
 
        ~_base() {
            // Éventuellement du code
        }
 
};
 
class uneClasse : private _base
{
 
    public:
        uneClasse(...) : _base(...) { // Un constructeur
             // Code spécifique
             ...
        }
 
        uneClasse(...) : _base(...) { // Un autre constructeur
             // Code spécifique
             ...
        }
 
        ...
 
};

Cas 2:

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
class uneClasse
{
 
    private:
        void init(/* paramètres éventuels */) {
            // Code partagé
            ...
        }
 
    public:
        uneClasse(...) { // Un constructeur
             init(...);
             // Code spécifique
             ...
        }
 
        uneClasse(...) { // Un autre constructeur
             init(...);
             // Code spécifique
             ...
        }
 
        ...
 
};

Merci !

[Astraya]

Bonjour ,

Cas 1

[Mat007]

Salut,

Aucun en ce qui me concerne, et je ne me souviens pas la dernière fois que j'ai senti le besoin de factoriser.
En général je trouve que c'est un code smell d'une classe trop grosse...

MAT.

[koala01]

Salut,

Le deuxième cas, sans hésitation...

Il n'y a aucune raison pour créer une classe de base si tu n'en a pas besoin pour autre chose.

N'oublie pas le fameux KISS (Keep It Simple, Stupid)

Ce que tu peux faire, sous réserve, c'est factoriser le code identique dans une fonction privée:

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
 
class UneClasse
{
    public:
        UneClass()
        {
            subinit(/* arguments éventuels*/);
            /* le reste */
        }
        UneClass(/* les arguments qui vont bien */)
        {
             subinit(/*arguments éventuels */);
             /* le reste */
        }
    private:
        subinit(/* arguments éventuels */)
        {
             /* ce qui doit être fait */
        }
};

Mais attention !!! La grosse réserve tient dans l'éventualité où "subinit" serait une fonction virtuelle et ou UneClasse servirait de base à d'autres classes.

En effet, ce serait le code relatif à UneClasse::subinit qui serait appelé, et non le code relatif à la redéfinition de cette fonction dans les classes dérivées, étant donné que, au moment où le constructeur de la classe de base est appelé, les classes dérivées ne sont pas encore construites

[Steph_ng8]

Salut,

Aucun en ce qui me concerne, et je ne me souviens pas la dernière fois que j'ai senti le besoin de factoriser.
En général je trouve que c'est un code smell d'une classe trop grosse...

MAT.

Ah oui tiens, je n'ai pas pensé à cette option dans le sondage...
Quelqu'un sait comment on peut le modifier ?

[Steph_ng8]

Salut,

Le premier cas, sans hésitation...

Il n'y a aucune raison pour créer une classe de base si tu n'en a pas besoin pour autre chose.

Bah le cas 1, c'est justement créer une classe de base...

Ce que tu peux faire, sous réserve, c'est factoriser le code identique dans une fonction privée:

Ça, c'est le cas 2...
C'est moi où il y a une problème de compréhension quelque part ?

Mais attention !!! La grosse réserve tient dans l'éventualité où "subinit" serait une fonction virtuelle et ou UneClasse servirait de base à d'autres classes.

En effet, ce serait le code relatif à UneClasse::subinit qui serait appelé, et non le code relatif à la redéfinition de cette fonction dans les classes dérivées, étant donné que, au moment où le constructeur de la classe de base est appelé, les classes dérivées ne sont pas encore construites

Ah non, dans la manière dont je vois les choses, cette fonction d'initialisation n'a aucune raison d'être virtuelle.
D'ailleurs, elle ne devrait être appelée que pour la création de la couche « uneClasse ».

[Steph_ng8]

Sinon, juste histoire d'être sûr, étant donné que « subinit » est privée, même si des classes filles définissent une fonction du même nom et pour le même usage, il n'y a pas de risque de conflit ?

[koala01]

Bah le cas 1, c'est justement créer une classe de base...


Ça, c'est le cas 2...
C'est moi où il y a une problème de compréhension quelque part ?

Ouppsss... c'est moi qui ai inversé les deux cas, je corrige mon intervention

Ah non, dans la manière dont je vois les choses, cette fonction d'initialisation n'a aucune raison d'être virtuelle.
D'ailleurs, elle ne devrait être appelée que pour la création de la couche « uneClasse ».

Nous sommes bien d'accord là dessus

[JolyLoic]

Ah oui tiens, je n'ai pas pensé à cette option dans le sondage...
Quelqu'un sait comment on peut le modifier ?

Je viens de le modifier pour toi... Fais moi signe si tu n'aimes pas l'intitulé choisi.

Sachant que j'ai un autre problème avec ta question de départ : Souvent, le corps de mes constructeurs est vide, et c'est la liste d'initialisation qui contient tout...

[Davidbrcz]

Perso j'attends C++0X et la délégation des constructeurs. Sinon, si j'ai déjà une classe de base, j'essaye de factoriser dans la classe de base, sinon j'essaye de voir pourquoi j'ai besoin de différents constructeurs et si je ne peux pas en éliminer (en utilisant des valeurs par défauts ....).

[hiura]

Perso j'attends C++0X et la délégation des constructeurs.

Serait-ce un système semblable à celui de Java ?

(Pour ceux qui ne connaisse pas : )

Code :

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

1
2
3
4
5
6
7
 
class B {
public B() {
  this("abc", 42); // Appel l'autre Ctor.
}
public B(String s, int i) {
  :

[Davidbrcz]

Serait-ce un système semblable à celui de Java ?

Regarde par toi même ><

http://www2.research.att.com/~bs/C++...elegating-ctor

[Steph_ng8]

Ah oui tiens, je n'ai pas pensé à cette option dans le sondage...
Quelqu'un sait comment on peut le modifier ?

Je viens de le modifier pour toi...

Ah, merci JolyLoic, c'est sympa.

Fais moi signe si tu n'aimes pas l'intitulé choisi.

« Ce genre de besoins semble louche » ?!?!
Hum, si tu veux...

[Steph_ng8]

Sachant que j'ai un autre problème avec ta question de départ : Souvent, le corps de mes constructeurs est vide, et c'est la liste d'initialisation qui contient tout...

Sinon, si j'ai déjà une classe de base, j'essaye de factoriser dans la classe de base, sinon j'essaye de voir pourquoi j'ai besoin de différents constructeurs et si je ne peux pas en éliminer (en utilisant des valeurs par défauts ....).

Alors je n'ai peut-être pas été assez clair, mais je pars du principe que le code a déjà été factorisé au maximum.
Autrement dit, tout ce qui peut être remonté dans les éventuelles classes-mère l'a été (à condition qu'elles soient accessibles), les paramètres par défaut sont utilisés au maximum pour réduire le nombre de constructeurs (quand cela est justifié), et tout ce qui peut être fait dans la liste d'initialisation ne l'est pas fait ailleurs.
Et malgré tout ça, il reste des bouts de code communs.

Ce n'est semble-t-il pas une situation que l'on rencontre souvent, mais une fois suffit pour se poser la question...

Bon, je crois qu'un cas pratique s'impose...
Imaginez que vous ayez deux constructeurs, l'un prenant un std::string comme paramètre et l'autre un char*.
Je vous entends hurler d'ici : « Vire le constructeur avec le char*, il y a la conversion implicite char* -> std::string. »
Fort bien, mais si par la suite on a besoin de travailler avec un char* (pour ouvrir un fichier, par exemple), pas sûr que ce soit le plus efficace d'allouer un std::string pour finalement aller rechercher les « données brutes », autant utiliser tout de suite le char*.

Et pourquoi ne pas faire comme la STL et ne faire qu'un constructeur prenant un char* comme paramètre ?
Pour simplifier la vie des gens qui vont utiliser ma classe (moi y compris), qu'ils puissent utiliser sans se poser de question un std:;string ou un char*.

Sinon, j'ai eu ce problème quand je faisais des fenêtres en Qt.
Je ne vais pas m'amuser à réécrire le code qui remplit la fenêtre dans tous les constructeurs que je ne peux pas fusionner, je ne peux pas le faire via la liste d'initialisation, et impossible de le faire remonter dans une classe-mère.

Voilà...
Ça vous paraît absurde comme motivation ?

[koala01]

En fait, d'expérience, il y a peu de cas dans lesquels il serait intéressant de convertir une std::string en const char * au niveau du constructeur:

Si c'est pour convertir la std::string en "autre chose" (par exemple en valeurs(s) numérique(s)), l'idéal sera quand même de passer par... un std::stringstream.

Si c'est pour manipuler un fichier, par exemple, l'idéal restera de n'ouvrir le fichier que "en cas de besoins" et de le garder "aussi peu" ouvert que possible, pour ne pas empêcher l'accès à ce fichier par ailleurs (et surtout par d'autres applications qui pourraient tourner en même temps que la tienne).

Il me semble même était question de voir de quelle manière supprimer les const char*de l'ensemble des fonctions et des classes qui les utilisent comme argument, mais je crois que, pour l'instant, le gros de l'effort porte sur la création de la liste exhaustive de celles-ci, et que cela pourrait provoquer certains problèmes de rétro compatibilité (à cause de l'opérateur de conversion implicite de const char * en std::string)

Quoi qu'il en soit, il me semble réellement préférable d'effectivement convertir les chaines "C style" "chaque fois que faire se peut" et de les maintenir, au sein de tes classes perso, sous la forme de std::string

Par contre, il reste vrai que certaines parties du constructeur peuvent malgré tout être factorisées une fois que tu as créé correctement tes listes d'initialisation: les contrôles de cohérence éventuels, par exemple, mais toujours en étant bien d'accord sur le fait que les fonctions factorisées ne doivent être effectuées que dans le cadre du type précis dont il est question, et en restant conscient du fait que cela *peut* poser des problèmes lors de l'héritage

[Klaim]

Perso j'attends C++0X et la délégation des constructeurs.

Aussi surprenant que cela paraisse, cette feature serait visiblement rééxaminée pour un potentiel retrait du standard : http://www.informit.com/guides/conte...lus&seqNum=504


Je ne sais pas clairement quels problemes la feature peut apporter, mais en tout cas je sais que ça serait sacrément dommage de ne pas l'avoir.

Contrairement à l'auteur de l'article, pour moi les valeurs d'initialization par défaut (dans la déclaration de classe) sont fondamentalement différents d'un constructeur commun. Ne serait-ce que parceque l'un des deux n'est pas du code executé.

[Steph_ng8]

Ce que je pensais ce confirme, à savoir qu'il vaut mieux écrire une fonction d'initialisation.

Merci à tous ceux et celles qui ont voté et/ou répondu !

[Mat007]

En fait il y a une autre possibilité, que pour le coup j'utilise très souvent.

Dans le fichier d'entête :

Code :

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

1
2
3
4
5
6
7
8
9
class uneClasse
{
 
    public:
        uneClasse(...); // Un constructeur
        uneClasse(...); // Un autre constructeur
    private:
        Bam bam_;
};

Et dans le fichier d'implémentation :

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
 
namespace
{
    Bam make_bam(...)
    {
        ......
        return bam;
    }
}
 
uneClasse::uneClasse( ... ) // un constructeur
 : bam_( make_bam( ... ) )
...
{}
 
uneClasse::uneClasse( ... )
 : bam_( make_bam( ... ) ) // un autre constructeur
...
{}

Donc avec des fonctions locales pour factoriser les créations/configurations de membres.

MAT.

[Steph_ng8]

Je vois...

Mais c'est vraiment nécessaire d'utiliser un attribut membre ?

[koala01]

Je vois...

Mais c'est vraiment nécessaire d'utiliser un attribut membre ?

Hé bien, tout dépend de ce que tu veux factoriser, mais, de manière générale, il y a trois choses que tu risques de factoriser:

1. La vérification des pré conditions
2. La vérification des post conditions
3. l'initialisation de membres, sur base de valeurs qui ne sont pas forcément du type du membre.

L'exemple que donne Mat est le troisième de la liste

Pour te donner un exemple concret des possibilités, imagine que tu crées une application"multi document", pour laquelle on peut créer un document sans lui donner de nom.

Le nom donné par défaut serait sans doute "Unnammed_XXX", et tu aura sans doute un gestionnaire de document (qui maintient l'ensemble des documents ouverts) qui ressemblera peu ou prou à quelque chose comme

Code :

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

1
2
3
4
5
6
7
8
9
10
class DocumentManager
{
    public:
        /* je passe les typdefs et les fonctions qui ne nous intéressent pas
         * ici
         */
        size_t count() const{return items_.size();}
    private : 
        LaCollection items_;
};

Ta classe document ressemblerait sans doute à

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
class Document
{
    public:
        /* le construteur créant un document anonyme */
        Document();
        /* et celui créant un document dont le nom est déjà connu */
        Document(std::string const & );
        /* sans oublier... l'accesseur sur le nom */
        std::string const & name() const{return name_;}
        /* et toutes les fonctions que je ne montre pas ici */
    private:
        std::string name_;
};

L'implémentation du constructeur par défaut ressemblerait donc à quelque chose comme

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
28
29
30
#include <document.hpp>
#include <documentmanager.hpp>
#include <sstream>
namespace
{
    void preConditions()
    {
         // les assertions qui vont bien avec les pré conditions 
    }
    std::string const doName()
    {
        preConditions();
        std::stringstream ss;
        ss<<"Unnamed_"<<DocumentManager::instance().count();
        return ss.str();
    }
    void postConditions()
    {
         if(il_y_a_eu_un_probleme)
             trhow LexceptionQuiVaBien();
    }
}
Document::Document():name_(doName())
{
    postConditions();
}
Document::Document(std::string const & n):name_(n)
{
    postConditions();
}