Discussion :

[Drife]

Bonjour à tous,

Dans un but purement d'entrainement, je code une classe pour gérer une liste chainée.

Pour cela j'ai choisi la technique du premier élément bidon (le premier élément n'en est pas un, c'est juste une accroche, créé lors de l'appel du constructeur).

Tout fonctionne pour le moment bien, mais j'aimerais votre avis sur mon destructeur...

Comme chaque élément ajouté dans la liste est créé dynamiquement avec un "new", j'avais besoin de gérer la libération de la mémoire dans le destructeur.

Une technique que j'ai trouvé est tout simplement de sauter le premier élément (bidon), puis dans le cas général :
sauvegarder l'élément suivant
appelé le destructeur dessus si il n'est pas nul

ça permet de parcourir et de détruire tous les éléments de la liste...

En exécution ça plante pas, en console j'ai une trace de la suppression de tous les éléments, bref ça à l'air de marcher.
Mais j'aimerai votre avis de pro.
Est-ce inutile, dangereux, idiot ?
Voici le code :

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
 
Noeud::~Noeud()
{
    Noeud *maillon_suivant = NULL;
    //premier appel du destructeur, depuis la tete de la liste, la valeur de this. On a pas besoin de faire un delete de cet élément, il n'a pas été alloué dynamiquement
    if ( this->est_accroche_liste() ){
        maillon_suivant = this->maillon_suivant;
        maillon_suivant->~Noeud();
    }
    //un appel après, sur un maillon quelquonque qui doit être supprimé
    else{
        cout << "Suppression de l'element "+ this->valeur << endl;
        maillon_suivant = this->maillon_suivant;//on sauve le suivant
 
        //Si le maillon suivant n'est pas à NULL (on n'est pas au bout de la chaine), on appel le destructeur sur le maillon suivant
        if( maillon_suivant != NULL){
            //cout << "Maillon suivant : "+ maillon_suivant->valeur << endl;
            delete maillon_suivant;
        }
    }
}

et le code complet de l'implémentation de la classe :

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
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
103
 
#include "../include/Noeud.h"
 
Noeud::Noeud(): valeur("NULL"), maillon_suivant(NULL)
{
    //cout << "Construction du premier element" << endl;
}
 
Noeud::Noeud(string s): valeur(s), maillon_suivant(NULL)
{
    //cout << "Construction de l\' element :"+s << endl;
}
 
Noeud::~Noeud()
{
    Noeud *maillon_suivant = NULL;
    //premier appel du destructeur, depuis la tete de la liste, la valeur de this. On a pas besoin de faire un delete de cet élément, il n'a pas été alloué dynamiquement
    if ( this->est_accroche_liste() ){
        maillon_suivant = this->maillon_suivant;
        maillon_suivant->~Noeud();
    }
    //un appel après, sur un maillon quelquonque qui doit être supprimé
    else{
        cout << "Suppression de l'element "+ this->valeur << endl;
        maillon_suivant = this->maillon_suivant;//on sauve le suivant
 
        //Si le maillon suivant n'est pas à NULL (on n'est pas au bout de la chaine), on appel le destructeur sur le maillon suivant
        if( maillon_suivant != NULL){
            //cout << "Maillon suivant : "+ maillon_suivant->valeur << endl;
            delete maillon_suivant;
        }
    }
}
 
bool Noeud::est_accroche_liste(){
    return valeur == "NULL";
}
 
bool Noeud::est_dernier_maillon(){
    return maillon_suivant == NULL;
}
 
void Noeud::ajout_au_bout(string valeur_a_ajouter){
    Noeud *noeud_a_ajouter = new Noeud(valeur_a_ajouter);
    Noeud* noeud_actuel = this;
 
    while( noeud_actuel->maillon_suivant != NULL){
        noeud_actuel = noeud_actuel->maillon_suivant;
    }
    noeud_actuel->maillon_suivant = noeud_a_ajouter;
}
 
void Noeud::ajout(string valeur_a_ajouter, int indice){
    int indice_depart = 1;
    Noeud *noeud_a_ajouter = new Noeud(valeur_a_ajouter);
    Noeud *noeud_actuel = this;
    Noeud *ancien_noeud_suivant = NULL;
 
    while( noeud_actuel->maillon_suivant != NULL && indice_depart < indice){
        noeud_actuel = noeud_actuel->maillon_suivant;
        indice_depart++;
    }
    //une fois le bon élément atteint, on insère le nouveau après cet élément, puis on le raccroche à la suite de la liste
    ancien_noeud_suivant = noeud_actuel->maillon_suivant;
    noeud_actuel->maillon_suivant = noeud_a_ajouter;
    noeud_a_ajouter->maillon_suivant = ancien_noeud_suivant;
}
 
string Noeud::get_valeur_noeud(int indice){
    int indice_actuel = 0;
    Noeud* noeud_actuel = this;
 
    while( noeud_actuel->maillon_suivant != NULL && indice_actuel < indice){
        noeud_actuel = noeud_actuel->maillon_suivant;
        indice_actuel++;
    }
    return noeud_actuel->valeur;
}
 
int Noeud::get_taille(){
    int taille = 0;
    Noeud* noeud_actuel = this;
 
    while( noeud_actuel->maillon_suivant != NULL){
        noeud_actuel = noeud_actuel->maillon_suivant;
        taille++;
    }
    return taille;
}
 
void Noeud::parcours(){
    Noeud* Noeud_actuel = this;
 
    if( est_accroche_liste()){
        Noeud_actuel = this->maillon_suivant;
    }
 
    while( Noeud_actuel->maillon_suivant != NULL ){
        cout << "Dans while " + Noeud_actuel->valeur << endl;
        Noeud_actuel = Noeud_actuel->maillon_suivant;
    }
    cout << "Dernière valeur : " + Noeud_actuel->valeur << endl;//On n'oublie pas d'afficher la dernière valeur
}

D'avance merci

PS : je sais bien sûr, y'a la STD et tout ça, c'est juste pour m'entrainer et bien comprendre

[Médinoc]

Ça me parait dangereux. Tu devrais plutôt créer un nouvel objet temporaire vide, et faire un non-throwing swap de leurs contenus. Ça évitera de risquer d'appeler deux fois le destructeur sur les mêmes données.

Ensuite, j'ai l'impression que ton problème c'est de confondre chaînon et liste. Je pense que tu devrais utiliser une classe Liste, qui posséderait un pointeur vers le Noeud bidon et s'occuperait de la destruction des Noeuds au lieu de faire en sorte que chaque Noeud tente de détruire le suivant.