Discussion :

[Jean-Pascal]

Bonjour développeurs.

Dans le but de créer une liste chaînée, je rencontre un problème concernant le destructeur d'une classe.
J'ai deux classes:
- Maillon qui contient un pointeur sur un autre maillon (le suivant) et une donnée
- Liste qui contient deux pointeurs sur des Maillons (le premier et le dernier)

Voici ma classe Maillon :

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
 
class Maillon
{
      friend class Liste;
      // Champs prives
      private:
        T _data ;
        Maillon *_suivant ;
 
      // Forme canonique de Coplien
      public:
        Maillon():_data(),_suivant(NULL){}
	 Maillon(const T& data):_data(data),_suivant(NULL){}
        Maillon(const Maillon& m):_data(m._data),_suivant(m._suivant){}
        ~Maillon() { delete _suivant; } // C'est ici que le problème se pose
 
      friend ostream& operator<< (ostream& o, Maillon& m);
};

Et voici ma classe Liste :

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
 
class Liste
{
    private:
        Maillon *_tete, *_fin ;
    public:
        Liste():_tete(NULL),_fin(NULL){}
        Liste(const Liste& l) ;
        ~Liste() // probleme : le destructeur appelle deux fois le destructeur d'un meme objet
       {
           Maillon *m = _tete;
           Maillon *tmp = new Maillon();
           while(m != NULL)
           {
                tmp = m;               //(1)
                m = m->_suivant;   //(2)
                delete tmp;           //(3)
           }
          _tete = NULL;
          _fin = NULL;
}

La première fois que l'on passe sur la ligne (3) on détruit l'objet tmp mais aussi son suivant. ensuite on repasse sur la ligne (1), tmp pointe alors sur un objet qui vient d’être détruit. Enfin, au deuxième passage sur la ligne (3), deuxième delete sur une même zone mémoire, le programme plante :

Process terminated with status -1073741571 (0 minutes, 3 seconds)

[quentinh]

Ton destructeur ~Liste() est très étrange…

Dans le cas de listes chaînées, le destructeur de Liste ne doit détruire que le premier objet (tete) et c'est tout…
Le destructeur de Maillon va supprimer le suivant (au passage, teste si _suivant est non NULL avant de le supprimer) qui va détruire le suivant, etc…

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
 
Maillon::~Maillon()
{
    if (_suivant != NULL) delete _suivant;
    _suivant = NULL;
}
 
Liste::~Liste()
{
    if (_tete != NULL) delete _tete;
    _tete = NULL; _fin = NULL;
}

[Steph_ng8]

Bonjour.

C'est une façon de faire.
Je ne sais pas si c'est la meilleure.

Mais ce n'est pas ça le plus étrange dans ton destructeur.

Code :

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

1
2
#line 12
Maillon *tmp = new Maillon();

Pourquoi donc construis-tu un nouveau maillon ???
Je suppose que tu voulais utiliser un pointeur temporaire, qui pointerait sur les maillons de la liste, afin de les détruire.

Il faut bien comprendre que, dans cas, on ne réserve pas une zone mémoire particulière pour ce pointeur ; il pointe sur des zonnes déjà allouées.
Donc il ne faut pas appeler new.
Surtout que toi tu pers l'adresse de la mémoire que tu viens d'allouer dès l'entrée dans la boucle...

Si tu veux qu'un maillon détruise son « suivant » lorsqu'il est lui-même détruit, reporte toi à la solution de quentinh ; les maillons se détruiront en cascade.
Sinon, le constructeur de Maillon n'a rien à faire, si ce n'est (éventuellement) mettre le champ _suivant à NULL, et le destructeur de Liste est correct, modulo ma remarque ci-dessus.

Selon tes besoins, tu peux définir une fonction membre qui retire le premier (et/ou dernier) élément de la liste, en faisant en sorte que les pointeurs _debut et _fin reste cohérents.
Du coup, le destructeur se contentera d'appeler cette fonction jusqu'à ce que la liste soit vide...

[Jean-Pascal]

D'accord, merci beaucoup!

[koala01]

Salut,

Personnellement, je ne crois pas qu'il soit intéressant de faire en sorte que le maillon détruise son suivant

La raison est simple : Tu peux, à un moment donné, décider de supprimer un maillon sans pour autant... suprimer celui qui le suit (et encore moins les suivants )

De plus, la responsabilité de la décision de la destruction d'un maillon n'a, a priori, aucune raison... d'incomber à un maillon

Enfin, le destructeur de ta liste boucle déjà sur tous les maillons pour les détruire... Cela signifie que, au mieux, la boucle de la liste est inutile, au pire, tu vas te retrouver à essayer de détruire deux fois certains maillons, et le programme ne le supportera pas

Par contre, étant donné que la responsabilité de la liste est la gestion des maillons, il semble tout à fait cohérent qu'elle s'occupe aussi... de les détruire (après tout, il faut essayer de garder un certain "parallélisme" : si la liste s'occupe de créer un maillon, pourquoi ne s'occuperait-elle pas de le détruire

Par contre, il faudra aussi très certainement t'intéresser à la problématique de la copie et de l'affectation! Telle que tu l'implémentes, tu armes un pistolet et tu le diriges dés à présent vers ton pied, pret à tirer

Il faut en effet s'assurer :

• Qu'il n'y ait aucun maillon qui, suite à une copie (une affectation) soit référencié comme suivant par deux maillons distincts car cela t'amènera fatalement à une tentative de double libération de la mémoire
• qu'il n'y aura pas de fuite mémoire si tu effectue l'affectation d'un maillon existant à un autre maillon existant

L'idiome copy and swap devrait t'intéresser pour éviter ces problèmes

Enfin, mais c'est la "tuch of class" , il serait pas mal de mettre en place un système d'itérateur : un objet qui permet de manipuler le pointeur sur maillon sous la forme d'une référence et qui permettra d'assurer la const-correctness du maillon

[3DArchi]

Salut

Je suis assez d'accord avec Koala. Il n'est pas très pertinent de faire détruire le pointeur dans le maillon. Comment feras-tu lorsque tu souhaiteras uniquement retirer un élément ?? La liste est clairement là pour gérer les différents maillons et entre autre les allouer et les libérer.

Autre problème : ta classe maillon est erronée. La copie telle quelle du pointeur dans le constructeur de copie Maillon(const Maillon& m):_data(m._data),_suivant(m._suivant){} est incompatible avec le destructeur ~Maillon() { delete _suivant; }. Si tu fais une copie d'un objet Maillon, le destructeur va être appelé deux fois.

Ensuite, si tu as un constructeur par copie, il serait assez logique de définir aussi l'opérateur d'affectation ! D'ailleurs, tu l'as (presque) marqué en commentaire dans ton code // Forme canonique de Coplien !!!

Ceci dit, une fois retirer la responsabilité de gestion du pointeur de Maille vers Liste, il deviendra assez naturel de ne garder que les versions implicites par défaut des destructeurs, constructeur par copie et opérateur d'affectation.