Salut, et bienvenue sur le forum.
Lorsque tu manipule des pointeurs, il faut savoir que la simple affectation de ceux-ci dans le constructeur par copie et l'opérateur d'affectation va avoir un effet un peu surprenant:
Les deux listes vont utiliser un seul et même élément (celui qui se trouve à l'adresse représentée par le pointeur de la première liste).
Or, il faut savoir qu'une variable qui n'a pas été créée en utilisant l'allocation dynamique (new) est systématiquement détruite (et son destructeur appelé) dés que l'on quitte la portée dans laquelle elle a été déclarée.
Ta manière d'implémenter les constructeurs par copie et opérateur d'affectation aura donc plusieurs effets pervers, qu'un peu de code me permettra de mettre en évidence:
1
2
3
4
5
6
7
8
9
10
11
| void foo()
{
MyLinkedList liste1;
/* remplissons la liste comme on veut */
if(test)
{
MyLinkedList liste2=liste1; /* (1) */
liste2.Add("toto"); /* (2) */
} /* (3) */
/* n'importe quelle manipulation de liste1 (4) */
} /* (5) */ |
Dans ce code, liste1 à considérer comme la liste "d'origine".
Tant que tu ne manipule qu'elle (avant d'arrriver au (1) ), il n'y a strictement aucun problème: tu travaille effectivement sur des éléments qui ne sont contenus que dans liste1.
Les problèmes vont commencer à se préparer dés que tu passe dans (1): en effet, liste1->m_start pointe sur le même élément que m2->m_start.
En (2), tu sera déjà face à un premier problème: l'ajout d'un élément dans liste2 sera répercuté dans liste1, or, si tu a décidé de travailler avec une autre liste, c'est clairement parce que tu souhaite travailler sur une liste qui contient des éléments identiques à ceux de liste1 mais sans aller modifier liste1.
Normalement, toute modification de liste2 dans le test devrait n'avoir aucun impact sur liste1
En (3) tu scelle ton sort parce que tu "casse tout":
Comme on sort de la portée dans laquelle liste2 est déclarée, liste2 est détruite, et son destructeur est appelé.
Tous les éléments de liste2 sont donc détruits, et, comme l'élément pointé par liste1->m_start est le même que celui pointé par liste2->m_strart, liste1->m_start pointe vers une adresse mémoire qui n'est plus utilisée.
Si tu manipule d'une quelconque manière liste1 après ce point par n'importe quelle utilisation de liste1 prenant place en (4), tu obtiendra une erreur de segmentation parce que liste1->m_start pointe sur une adresse qui est devenue invalide.
Si tu n'essaye pas de manipuler liste1, tu ne sera pas sauvé pour la cause.
En effet, comme on quitte, en (5) la portée dans laquelle liste1 est déclarée, la variable est détruite, et son destructeur est appelé automatiquement.
Le destructeur va donc essayer de libérer une deuxième fois l'adresse mémoire représentée par liste1->m_start (la première fois que cette adresse mémoire a été libérée est survenue dans (3) ) et, fatalement, tu obtiendra une là aussi une erreur de segmentation.
La solution à tous ces maux est de faire en sorte que la copie de la liste contienne... une copie de tous éléments de la liste originale.
Le constructeur par copie va donc prendre une forme proche de
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
| MyLinkedList::MyLinkedList(const MyLinkedList& other) : m_start(0) /* ( 1 ) */
{
/* un pointeur "temporaire" permettant de parcourir other */
node * tempRead = other.m_start;
/* un pointeur temporaire permettant de garder la référence
* sur le dernier élément de la liste que l'on remplit
*
* cela nous permettra d'éviter de parcourir chaque fois toute
* la liste pour le retrouver
*/
node *last = NULL;
/* tant que nous trouvons un élément dans other */
while(tempRead)
{
/* Nous en créons une copie qui prend la valeur de l'élément
* en cours dans other et dont le pointeur next pointe sur NULL
* ( 2 )
*/
node *temp=new node;
temp->next =NULL;
temp->str = tempRead->str;
/* nous insérons ce nouvelle élément dans la liste que l'on remplit
*
* c'est peut-être le premier élément que l'on ajoute
*/
if(!m_start)
m_start= temp;
/* si nous avons déjà défini un dernier élément (autrement dit,
* si l'élément que l'on ajoute n'est pas le premier élément)
* nous relions le dernier élément à celui fraichement créé
*/
if(last)
last->next = temp;
/* l'élément fraichement créé devient celui auquel il faudra
* relier le suivant
*/
last = temp;
/* et nous passons à l'élément suivant dans other */
tempRead=tempRead->next; /* ( 3 ) */
}
} |
Tu l'aura remarqué, j'ai marqué trois points particuliers qui nécessitent une petite explication.
( 1 ) : dans les constructeurs, il est souvent préférable d'initialiser les membres dans ce que l'on appelle un "liste d'initialisation".
Cette liste d'initialisation prend place juste après la parenthèse fermante de la liste des arguments, commence par le symbole deux points " : " et est composée de l'appel d'un constructeur (par défaut ou non, voire par copie) de tous les membres de la classe, séparés par une virgule.
Cela permet d'initialiser directement et de manière correcte les différents membres et résous deux problèmes particuliers:
Le premier est qu'il est possible qu'il faille passer un (ou plusieurs) arguments à l'un des membre lors de sa construction (qui a, justement, lieu entre la parenthèse fermante de la liste des argument et l'accolade ouvrante du début de la définition du constructeur)
Le second est que la création et la copie d'objets prennent parfois du temps et des ressources.
L'utilisation éventuelle du constructeur par copie permet de "tout faire d'un coup", alors que si tu écris un code proche de
1
2
3
4
| MaClass::MaClass(MaClass const & other)
{
membre=other.membre;
} |
tu aura la création par défaut d'un objet correspondant à membre (entre la parenthèse fermante et l'accolade ouvrante) suivie (dans le code) par... une affectation du membre, ce qui revient à faire finalement deux fois le travail 
Concernant les point ( 2 ) et ( 3 ), je voudrais signaler que la seule différence entre une classe et une structure tient dans la visibilité par défaut des membres.
Elle est en private: pour les classes et en public: pour les structures.
A part ce détail, tout ce qui s'applique aux classes s'applique aux structure, et il y a donc création automatique de constructeurs par défaut et par copie, d'un opérateur d'affectation et d'un destructeur si tu ne les déclares pas toi-même, tant pour les classes que pour les structures:- Le constructeur par défaut va initialiser les membres en invoquant leurs constructeur par défaut
- Le constructeur par copie va initialiser les membre en invoquant leurs constructeur par copie
- L'opérateur d'affectation affectera à la structure ou à la classe une copie de tous les membre de la classe ou de la structure
- Le destructeur appellera automatiquement le destructeur de tous les membres pour lesquels la mémoire n'a pas été allouée dynamiquement
Le fait est que, un pointeur représentant en réalité une adresse mémoire, les constructeur par copie et opérateur d'affectation vont, tout simplement, copier l'adresse d'un membre à l'autre, et non effectuer une copie "en profondeur" (en somme, les comportements que tu as défini pour ton constructeur par copie et ton opérateur d'affectation sont fort proches des comportements implémentés par le compilateur)
Et bien sur, une même cause ayant les mêmes effets, si tu laisse ces comportements par défaut, tu risque de te trouver confronté à des problèmes dés qu'il s'agit de gérer la copie de membres qui sont en réalité des pointeurs
De plus, même pour les structures, il est souvent intéressant de penser avec l'idiome de RAII (Ressource Acquisition Is Initialization), c'est à dire de faire en sorte à ce que la construction (qu'elle se fasse à l'aide de new ou non) d'un objet te permette d'obtenir directement un objet correctement initialisé.
Pour ton type node, cela passe par le fait de s'assurer que, tant lors de la création que lors de la copie, le pointeur next soit initialisé à ... NULL.
Cette aparté étant faite.
Nous avons maintenant donc la possibilité de copier correctement notre liste, c'est à dire qu'un code proche de
1
2
3
| MyLinkedList liste1;
/* on remplit liste1 */
MyLinkedList liste2(liste1); |
te permet effectivement de manipuler deux listes contenant des éléments équivalents (dans le sens ou les valeurs de str sont respectivement identiques) mais utilisant des adresses mémoires clairement différentes.
Il nous reste le problème de l'affectation à gérer.
Une même cause ayant les mêmes effets, le code que tu présente pour ton opérateur d'affectation fera que chaque fois que tu écrira un code proche de
1
2
3
4
|
MyLinkedList temp;
/*...*/
temp = liste1; |
temp->m_start et liste1->m_start pointeront également vers la même adresse mémoire, alors que ce n'est, on l'a déjà constaté, pas souhaitable.
On pourrait contourner ce problème en envisageant une écriture proche de
temp=MyLinkedList(liste1);
mais il faut avouer que ce n'est pas vraiment une écriture intuitive.
Pire, cela nous laisserait face à un autre problème bien plus important.
En effet, si l'on en vient à écrire un code proche de
1
2
3
4
5
6
7
| temp = MyLinkedList(liste1); /* s'assure que l'on travaille sur une copie de
* liste1)
*/
/* ... */
temp = MyLinkedList(liste2); /* s'assure que l'on travaille sur la copie
* d'une autre liste
*/ |
temp n'est pas détruite entre les deux lignes significatives vu que l'on ne sort pas de sa portée, et le destructeur n'est donc pas appelé...
lorsque l'on arrive à la deuxième ligne significative, nous perdons l'adresse mémoire de la copie du premier élément de liste1 pour la remplacer par l'adresse de la copie du premier élément de liste2, et cela occasionne une fuite mémoire...
Il faut donc veiller à ce que le contenu de temp soit correctement détruit avant d'assigner le contenu de la copie de liste2 mais aussi s'assurer de ne pas détruire le contenu de temp si, pour une raison ou une autre, la copie de liste2 échoue.
La solution à ce double problème passe par un idiôme que l'on appelle copy and swap, que l'on pourrait traduire par "copier et intervertir".
L'idée générale est de provoquer une copie de la liste à affecter avant d'intervertir les membres de liste de destination et de cette copie pour enfin être en mesure de détruire la copie (qui contiendra alors ... le contenu originel de la liste de destination.
L'opérateur d'affectation prendra alors une forme proche de
1
2
3
4
5
6
7
8
9
10
11
12
| MyLinkedList& operator=(MyLinkedList const & other)
{
MyLinkedList temp(other); /* on crée une copie de la liste à affecter */
/* on interverti les membre de l'instance en cours et de la copie
* (1)
*/
std::swap(m_start,temp.m_start);
/* on renvoie l'instance en cours */
return *this;
} /* temp est automatiquement détruite, et, avec elle, ce qui était le
* contenu de l'instance en cours avant l'interversion
*/ |
( 1 ) std::swap est une fonction fournie par le standard et disponible dans le fichier d'en-tête <algorithm> qui permet d'intervertir deux choses de type identique.
Son comportement est tout à fait identique à ce que l'on obtiendrait avec des code proches de
1
2
3
| node* n = temp.m_start;
temp.m_start = m_start;
m_start = temp; |
ou de
1
2
3
| m_start ^ temp.m_start;
temp.m_start ^ m_start;
m_start ^ temp.m_strat; |
tout en nécessitant moins de ligne et en étant, sommes toutes plus explicite 
On peut d'ailleurs noter qu'il serait peut être intéressant de permettre à une liste d'être intervertie avec une autre
Maintenant que j'ai répondu à ton problème technique, je ne peux m'empêcher de te signaler qu'il existe une liste fournie par le standard, et qui te facilitera énormément la tâche en gérant tout cela pour toi.
Il s'agit de la classe... list, disponible dans l'espace de noms std par simple inclusion du fichier d'en-tête <list>
copy constructor/ assignement/ destructor
Répondre avec citation
A méditer: La solution la plus simple est toujours la moins compliquée
Partager