[C++] Demande précision sur Pattern Decorateur

Version imprimable

Bonjour,

Après le pattern Factory, je me pose des questions sur le pattern Decorateur ;)

Déjà sur le principe : On utilise ce pattern quand on veut "greffer" à un objet des caractéristiques plus complexes ou modifier certaines de ses caractéristiques de base. En assemblant, plusieurs décorateurs, on peut obtenir un objet complexe. C'est bien cela ? :oops:

Ensuite sur son implémentation :
Soit une classe de base Enemy. Le but est de créer toutes sortes d'ennemies ayant des caractéristiques différentes (vol, feu, plus de PV, plus puissant, ...)
Le première idée est d'utiliser l'héritage classique + polymorphisme avec un truc du genre :
Code:

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 Enemy { virtual void attaquer() = 0; }; class FlyEnemy : public Enemy { void attaquer() {cout << "j'attaque en volant" << endl} }; class PowerEnemy : public Enemy { void attaquer() {cout << "j'attaque avec toute ma puissance..." << endl} }; int main { std::vector<Enemy*> e; e.pushBack(new FlyEnemy()); e.pushBack(new PowerEnemy()); for(chaque ennemies du tableau) { e->attaquer(); } }
Voilà je me demandais si on pouvait améliorer cette méthode par le pattern Decorateur ?

Merci pour vos conseils ;)

24/08/2011, 19h17
Flob90

Bonsoir,

L'objectif du pattern décorateur est le suivant : permettre l'ajout dynamique d'un ensemble de comportement à des objets.

Il ne peut s'utiliser dans le même cas que l'héritage (comme tu le fais dans ton exemple), du moins il ne devrait pas : les cas d'utilisation sont distincts.

Je vais essayer de reprendre ton exemple. Imaginons qu'on ai des ennemis qui peuvent être munis de différentes compétences : FireSkill, ColdSkill, PowerSkill, ect. Ainsi on peut voir un ennemi particulier comme un ennemi de base (Ennemy) auquel on ajoute divers comportements selon les compétences qu'ils possèdents. Les classes ...Skill vont donc contenir tout ce qu'il faut pour offrir les comportements liés à chaque type de compétence.

Imagines que tu réalises ceci avec l'héritage et que tu ais beaucoup de compétences différentes. Déjà rien qu'avec ces 3 types de compétences on tombes à 3^2 = 9 classes différentes ! Je te laisse imaginer si le nombre de types de compétences est plus important.

Alors que si les classes ...Skill sont des décorateurs de la classe Ennemy le système devient plus flexible : il n'y a pas de création d'un nombre important de classe, la création d'un ennemie n'est que la décoration via les classes ...Skill (qui seraient donc les décorateurs) d'un objet de type Ennemy. L'ajout des compétences de chaques créature devient dynamique et plus statique.

Correction : 2^3 = 8 classes pas 9 :)
25/08/2011, 11h44
oodini

Citation:

Envoyé par Flob90

L'objectif du pattern décorateur est le suivant : permettre l'ajout dynamique d'un ensemble de comportement à des objets.

Et comparé au Stratégie ?

Ok pour ces précisions.

Le nouveau code devient :

Code:

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
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
#include <iostream>
 
using namespace std;
 
class BaseEnemy
{
    protected:
        int health;
        int defensive;
        std::string name;
 
        BaseEnemy(int _h, int _d, std::string _n)
            : health(_h), defensive(_d), name(_n) {};
        ~BaseEnemy();
 
    public:
        virtual int getHealth() const {return health;};
        virtual int getDefensive() const {return defensive;};
        virtual std::string getName() const {return name;};
 
        virtual void getInformations() const
        {
            std::cout << "Vie : " << health << std::endl;
            std::cout << "Defense : " << defensive << std::endl;
            std::cout << "Nom : " << name << std::endl << std::endl;
        };
};
 
class Enemy : public BaseEnemy
{
    public:
        Enemy(int _h, int _d, std::string _n)
            : BaseEnemy(_h,_d, _n) {};
        ~Enemy(){};
};
 
class Decorator : public BaseEnemy
{
    protected:
        BaseEnemy* base;
        Decorator(BaseEnemy* _base, int _h, int _d, std::string _n)
            : BaseEnemy(_h, _d, _n), base(_base) {};
        ~Decorator() {};
 
    public:
        BaseEnemy* getBase() const {return base;};
};
 
class PowerSkill : public Decorator
{
    public:
        int getHealth() const {return base->getHealth()+health;};
        int getDefensive() const {return base->getDefensive()+defensive;};
        std::string getName() const {return base->getName()+" "+name;};
 
        void getInformations() const
        {
            std::cout << "Vie : " << getHealth() << std::endl;
            std::cout << "Defense : " << getDefensive() << std::endl;
            std::cout << "Nom : " << getName() << std::endl << std::endl;
        };
 
        PowerSkill(BaseEnemy* _base, int _h, int _d, std::string _n)
            : Decorator(_base, _h, _d, _n) {};
        ~PowerSkill() {};
};
 
class FireSkill : public Decorator
{
    private:
        int powerFire;
 
    public:
        int getPowerFire() const {return powerFire; };
 
        void getInformations() const
        {
            base->getInformations();
            std::cout << "Puissance de feu : " << getPowerFire() << std::endl;
        };
 
        FireSkill(BaseEnemy* _base, int _powerFire)
            : Decorator(_base, 0, 0, ""), powerFire(_powerFire) {};
        ~FireSkill() {};
};
 
int main()
{
    BaseEnemy* b = new Enemy(5, 10, "Enemy");
    b->getInformations();
 
    b = new PowerSkill(b, 2, 3, "Power Enemy");
    b->getInformations();
 
    b = new FireSkill(b, 100);
    b->getInformations();
 
  //  delete b;
 
    return 0;
}

Est ce la bonne implémentation du pattern ?

J'ai vu que certain utilisait la base par référence, j'ai utilisé un pointeur je pense que ca ne change rien ?

Et dernière chose, mon code a des fuites de mémoire et je n'arrive pas à libérer intelligemment la mémoire :(

Citation:

Envoyé par Aspic

J'ai vu que certain utilisait la base par référence, j'ai utilisé un pointeur je pense que ca ne change rien ?

Le décorateur a une durée de vie inférieur à la base et il n'est pas sensé la gérer. Le décorateur n'existe pas sans base. La référence permet de garantir ça puisqu'une référence ne peut être null contrairement à un pointeur

Citation:

Envoyé par Aspic

Et dernière chose, mon code a des fuites de mémoire et je n'arrive pas à libérer intelligemment la mémoire :(

Et oui car les durées de vie des décorateurs et des bases sont indépendantes. Il te faut donc les gérer à part explicitement. D'où une utilisation plus facile avec des références :)
cf tuto de david
Code:

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 class Decorator : public BaseEnemy { protected: BaseEnemy& base; Decorator(BaseEnemy& _base, int _h, int _d, std::string _n) : BaseEnemy(_h, _d, _n), base(_base) {}; ~Decorator() {}; public: BaseEnemy& getBase() const {return base;}; }; void getInformations(BaseEnemy &b) { b.getInformations(); } int main() { BaseEnemy b(5, 10, "Enemy"); getInformations(b); PowerSkill ps(b, 2, 3, "Power Enemy"); getInformations(ps); FireSkill fs(ps, 100); getInformations(fs); return 0; }
[EDIT] : si tu voulais gérer ça par pointeur, dans le décorateur ce serait plus probablement un std::weak_ptr plutôt qu'un std::shared_ptr puisque les durées de vie n'ont pas à être liées.

Ok pour la référence ;)
J'ai modifié mon code en conséquence mais ca fuit toujours :

Citation:

Envoyé par rapport de valgrind

--3140-- REDIR: 0x41db3b0 (free) redirected to 0x4025b6b (free)
==3140==
==3140== HEAP SUMMARY:
==3140== in use at exit: 78 bytes in 4 blocks
==3140== total heap usage: 9 allocs, 5 frees, 214 bytes allocated
==3140==
==3140== Searching for pointers to 4 not-freed blocks
==3140== Checked 101,764 bytes
==3140==
==3140== 78 (20 direct, 58 indirect) bytes in 1 blocks are definitely lost in loss record 4 of 4
==3140== at 0x402641D: operator new(unsigned int) (vg_replace_malloc.c:255)
==3140== by 0x8048B50: main (in /mnt/partage/decorateur_ennemie/test)
==3140==
==3140== LEAK SUMMARY:
==3140== definitely lost: 20 bytes in 1 blocks
==3140== indirectly lost: 58 bytes in 3 blocks
==3140== possibly lost: 0 bytes in 0 blocks
==3140== still reachable: 0 bytes in 0 blocks
==3140== suppressed: 0 bytes in 0 blocks
==3140==
==3140== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 17 from 6)
--3140--
--3140-- used_suppression: 17 U1004-ARM-_dl_relocate_object
==3140==
==3140== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 17 from 6)

Code :

Code:

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
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
#include <iostream>
 
using namespace std;
 
class BaseEnemy
{
    protected:
        int health;
        int defensive;
        std::string name;
 
        BaseEnemy(int _h, int _d, std::string _n)
            : health(_h), defensive(_d), name(_n) {};
 
    public:
        ~BaseEnemy() {};
 
        virtual int getHealth() const {return health;};
        virtual int getDefensive() const {return defensive;};
        virtual std::string getName() const {return name;};
 
        virtual void getInformations() const
        {
            std::cout << "Vie : " << health << std::endl;
            std::cout << "Defense : " << defensive << std::endl;
            std::cout << "Nom : " << name << std::endl << std::endl;
        };
};
 
class Enemy : public BaseEnemy
{
    public:
        Enemy(int _h, int _d, std::string _n)
            : BaseEnemy(_h,_d, _n) {};
        ~Enemy(){};
};
 
class Decorator : public BaseEnemy
{
    protected:
        BaseEnemy& base;
        Decorator(BaseEnemy& _base, int _h, int _d, std::string _n)
            : BaseEnemy(_h, _d, _n), base(_base) {};
        ~Decorator(){  };
 
    public:
        BaseEnemy& getBase() const {return base;};
};
 
class PowerSkill : public Decorator
{
    public:
        int getHealth() const {return base.getHealth()+health;};
        int getDefensive() const {return base.getDefensive()+defensive;};
        std::string getName() const {return base.getName()+" "+name;};
 
        void getInformations() const
        {
            std::cout << "Vie : " << getHealth() << std::endl;
            std::cout << "Defense : " << getDefensive() << std::endl;
            std::cout << "Nom : " << getName() << std::endl << std::endl;
        };
 
        PowerSkill(BaseEnemy& _base, int _h, int _d, std::string _n)
            : Decorator(_base, _h, _d, _n) {};
        ~PowerSkill() {};
};
 
class FireSkill : public Decorator
{
    private:
        int powerFire;
 
    public:
        int getPowerFire() const {return powerFire; };
 
        void getInformations() const
        {
            base.getInformations();
            std::cout << "Puissance de feu : " << getPowerFire() << std::endl;
        };
 
        FireSkill(BaseEnemy& _base, int _powerFire)
            : Decorator(_base, 0, 0, ""), powerFire(_powerFire) {};
        ~FireSkill() {};
};
 
int main()
{
    BaseEnemy* b = new Enemy(5, 10, "Enemy");
    b->getInformations();
 
    b = new PowerSkill(*b, 2, 3, "Power Enemy");
    b->getInformations();
 
    b = new FireSkill(*b, 100);
    b->getInformations();
 
    delete b;
 
    return 0;
}

J'ai été obligé de mettre le destructeur en public dans BaseEnemy pour faire un delete b; dans le main.

Mais d'un autre côté ca ne m'étonne pas que ca fuit vu que j'ai 3 new pour un seul delete 8O

25/08/2011, 13h47
dragonjoker59

Mets tes destructeurs en virtual.
Là le destructeur de BaseEnemy ne doit jamais être appelé, si je ne me trompe pas.
Ensuite, effectivement, 3 new pour un seul delete ... :aie:
25/08/2011, 14h00
Aspic

Citation:

Envoyé par dragonjoker59

Mets tes destructeurs en virtual.
Là le destructeur de BaseEnemy ne doit jamais être appelé, si je ne me trompe pas.
Ensuite, effectivement, 3 new pour un seul delete ... :aie:

Ca ne change rien :(

Et si je met le destructeur en protected, je ne peux plus faire de delete et donc j'ai 3 new pour aucun delete !!! :(
25/08/2011, 14h01
Flob90

@dragonjoker59: C'est surtout que ca risque de finir en UB si on ne mets pas les destructeur des classes de bases en virtuel (ou en section protégé selon ce qu'on veut faire).

@Aspic: Dans ton code tu n'as aucun élément qui a la repsonsabilité de gérer la durée de vie de tes objet, donc la règles devient simple : une new = un delete. Delete les décorateurs que tu as utilisé et tu n'auras plus de fuite. Si la situation devient plus complexe tu seras surment amener à introduire dans ton architecture quelque chose qui aura la responsabilité de ces objets.

@oodini: Le attern stratégie n'a pas pour but d'ajouter des comportements aux objets. Son objectif est plutôt de permettre l'utilisation d'un point de variation d'un algorithme, ie on a un seul objectif mais on peut choisir entre plusieurs facon de l'atteindre grace à ce pattern.
25/08/2011, 14h04
Aspic

Citation:

Envoyé par Flob90

@Aspic: Dans ton code tu n'as aucun élément qui a la repsonsabilité de gérer la durée de vie de tes objet, donc la règles devient simple : une new = un delete. Delete les décorateurs que tu as utilisé et tu n'auras plus de fuite. Si la situation devient plus complexe tu seras surment amener à introduire dans ton architecture quelque chose qui aura la responsabilité de ces objets.

Sauf que je ne peux pas faire de delete puisque le destructeur de BaseEnemy est en protected.

De plus, je ne peux pas faire 3 delete car je rajoute des décorateurs au même objet de type BaseEnemy.
25/08/2011, 14h24
Flob90

C'est un oublie de l'article de David (son code ne libère pas la mémoire qu'il alliue, et avec ce destructeur en protégé il ne peut pas le faire), le destructeur de BaseEnnemy ne doit pas être protégé si ensuite on manipule des objets alloués dynamiquement dont on a la responsabilité via des BaseEnnemy* (pour faire le delete comme tu l'as remarqué).

Si tu fais les deletes dans le bon ordre il n'y a pas de problème, détruire un décorateur ne détruit pas l'objet qu'il décore d'après ton code.

Citation:

Envoyé par Flob90

C'est un oublie de l'article de David (son code ne libère pas la mémoire qu'il alliue, et avec ce destructeur en protégé il ne peut pas le faire), le destructeur de BaseEnnemy ne doit pas être protégé si ensuite on manipule des objets alloués dynamiquement dont on a la responsabilité via des BaseEnnemy* (pour faire le delete comme tu l'as remarqué).

Si tu fais les deletes dans le bon ordre il n'y a pas de problème, détruire un décorateur ne détruit pas l'objet qu'il décore d'après ton code.

Ok donc je remet le destructeur en public.

Mais je en peux pas faire ca:
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 int main() { BaseEnemy* b = new Enemy(5, 10, "Enemy"); b->getInformations(); delete b; b = new PowerSkill(*b, 2, 3, "Power Enemy"); b->getInformations(); delete b; b = new FireSkill(*b, 100); b->getInformations(); delete b; return 0; }
Déjà ca ne marche pas et puis il faut le même objet pour appliquer les décorateurs donc je ne peux pas le détruire à chaque fois.

Mais vu que je n'ai qu'un seul pointeur dans le main sur BaseEnemy, je ne vois pas comment accéder aux destructeurs de FireSkill et PowerSkill...

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
 
template<class BaseType>
BaseType* remove_deco(BaseType* b)
{
  if(Decorator* d = dynamic_cast<b>)
  {
    BaseType* tmp = d->getBase();
    swap(tmp,b); delete tmp;
  }
  return b;
}
 
template<class BaseType>
void delete_objet(BaseType* b)
{
  BaseType* tmp = remove_deco(b);
  while(b != tmp)
  {
    b = tmp;
    tmp = remove_deco(b);
  }
  delete b;
}

Quelque chose dans ce gout là.

Oublie pas de mettre les destructeurs en virtuel (celui de la base suffit, le caractère virtuel se transmet).

26/08/2011, 12h53
Aspic

C'est un peu "crade" de faire un dynamic_cast ?
Ca me parait bizarre.

SInon ca semble fonctionner ;)
26/08/2011, 15h31
Emmanuel Deloget
Citation:

Envoyé par Aspic

C'est un peu "crade" de faire un dynamic_cast ?

C'est un cas un peu limite. Son problème est plus qu'il introduit un couplage entre la classe de base et la classe de décoration. Personnellement, j'aurais fait autrement :
Code:

1 2 3 4 5 6 7 8 9 10 11 12 class decorator : public base { base *b; public: virtual ~decorator() { if (dynamic_cast<decorator*>(b)) { delete b; } } };
De fait, je n'introduit aucun couplage, puisque je suis déjà sur que ce couplage existe à cet endroit précis. On peut remplacer le dynamic_cast par un appel à une méthode virtuelle de b (par exemple: virtual bool destroy_on_destruction() const, qui renvoie false dans la classe base et true dans la classe decorator). C'est moins joli, mais ça devient une nécessité si d'autres conditions peuvent intervenir.
26/08/2011, 17h46
Flob90

@Emmanuel Deloget: Mon code avait pour but de retirer une décoration, pas de tout supprimer (*). Et j'était resté sur l'idée de ne pas introduire de gestion de durée de vie dans les objets de base et les décorateurs, mais ce n'est qu'un choix.

(*) Pour la fonction qui contient le dynamic_cast.
26/08/2011, 22h02
Aspic

Je suis pas trop pour les cast dynamiques mais bon, avec ce pattern ca semble la seule solution pour éviter les fuites de mémoire ;)