IdentifiantMot de passe
Loading...
Mot de passe oublié ?Je m'inscris ! (gratuit)
Navigation

Inscrivez-vous gratuitement
pour pouvoir participer, suivre les réponses en temps réel, voter pour les messages, poser vos propres questions et recevoir la newsletter

 C++ Discussion :

copy constructor/ assignement/ destructor


Sujet :

C++

Vue hybride

Message précédent Message précédent   Message suivant Message suivant
  1. #1
    Futur Membre du Club
    Inscrit en
    Août 2009
    Messages
    3
    Détails du profil
    Informations forums :
    Inscription : Août 2009
    Messages : 3
    Par défaut copy constructor/ assignement/ destructor
    Bonjour,

    Je travaille sur des listes chainees en c++.

    J ai mon fichier MyLinkedList.h tel que:

    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
    #ifndef MYLINKEDLIST_H
    #define MYLINKEDLIST_H
     
    #include <iostream>
    #include <string>
     
    class MyLinkedList{
    	private:
    		struct node 
    		{
    			std::string str;
    			node *next;
    		} *m_start;
     
    	public:
    		MyLinkedList(); //default constructor
    		MyLinkedList(const MyLinkedList& other);  //copy constructor
    		MyLinkedList operator=(const MyLinkedList& other);
    		//void Add(const std::string& s );
    		//bool Remove(const std::string& s);
    		//bool InList(const std::string& s) const;
    		//void PrintList() const;
    		//int Count() const;
    		//void Sort(); 
    		~MyLinkedList(); // destructor
    };
     
    #endif
    L implementation des constructeurs, de l operateur = et du destructeur a lieu dans le fichier MyLinkedList.cpp et est telle que:

    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
    MyLinkedList::MyLinkedList()
    {
    	m_start=NULL;
    }
     
    MyLinkedList::MyLinkedList(const MyLinkedList& other)
    {
    	m_start= other.m_start;
    }
     
    MyLinkedList MyLinkedList::operator=(const MyLinkedList& other)
    {
    	m_start= other.m_start;
    	return *this;
    }
     
     
    MyLinkedList::~MyLinkedList()
    {
    	if(m_start==NULL)
    		return;
     
    	while(m_start!=NULL)
    	{
    		node *temp;
    		temp = m_start->next;
    		delete m_start;
    		m_start=temp;
    	}
    }
    Si j utilise le constructeur par defaut, tout va bien mais si j utilise le constructeur de copie, je recois une segmentation fault.
    Avec valgrind j ai verifie que toute la memoire n est pas liberee a la fin, j ai donc un probleme dans mon destructeur que je n arrive pas a localiser.

    Qqn a t il une idee?

    Merci !!!

  2. #2
    screetch
    Invité(e)
    Par défaut
    Il manque l'implémentation des fonctions, du coup "new" n'est jamais appelé
    mais surtout tes listes chainées partagent la "vraie" liste chainée (la liste de "node") si tu fais par exemple :

    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    6
    MyLinkedList l;
    l.Add("toto");
    l.Add("titi");
    MyLinkedList l2 = l; // ici, l2 et l sont les listes toto->titi
    // lorsque l sera détruit, il va detruire la liste chainée
    // lorsque l2 sera detruit, il va AUSSI detruire la liste chainée
    de plus, lorsque tu appelles l'operateur =, il ne desalloue pas la liste précédente
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    MyLinkedList l;
    l.Add("toto");
    l.Add("titi");
    MyLinkedList l2;
    l = l2; // qu'advient il des anciennes données pointées par l

  3. #3
    Futur Membre du Club
    Inscrit en
    Août 2009
    Messages
    3
    Détails du profil
    Informations forums :
    Inscription : Août 2009
    Messages : 3
    Par défaut Re
    Voici l implementation des fonctions (sauf Sort() ). Avec l appel de new...


    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
    void MyLinkedList::Add(const string& s)
    {
    	if(InList(s) == false)
    	{
    		if(m_start==NULL)
    		{ 
    			m_start=new node;
    			m_start->str=s;
    			m_start->next=NULL;
    		}
    		else
    		{
    			node *temp1;
    			node *temp2;
     
    			temp1= m_start;
    			while(temp1->next!=NULL)
    				temp1 = temp1->next;
     
    			temp2= new node;
    			temp2->str= s;
    			temp2->next=NULL;
    			temp1->next=temp2;
    		}
    	}
    	//else
    	//	return;
    }
     
    bool MyLinkedList::Remove(const string& s)
    {
    	node *temp1;
    	node *temp2;
     
    	if(m_start == NULL)
    	{
    		return false;
    	}
     
    	temp1 = m_start;
    	if(temp1->str == s)
    	{
    		m_start=temp1->next;
    		delete temp1;
    		return true;
    	}
     
    	temp2 = temp1;
    	while(temp1 != NULL)
    	{
    		if(temp1->str == s)
    		{
    			temp2->next = temp1->next;
    			delete temp1;
    			return true;
    		}
    		temp2 = temp1;
    		temp1 = temp1->next;
    	}
     
    	return false;
    }
     
    bool MyLinkedList::InList(const string& s) const
    {
    	node *temp = m_start;
     
    	while(temp != NULL)
    	{
    		if(temp->str == s){
    			return true;
    		}
     
    		temp = temp->next;
    	}
     
    	return false;
    }
     
    void MyLinkedList::PrintList()const
    {
    	if(m_start==NULL)
    		cout<<"EMPTY"<<endl;
    	else
    	{
    		node* temp;
    		for(temp =m_start; temp !=NULL; temp=temp->next)
    			cout<<temp->str<<endl;
    		//std::cout<<std::endl;
    	}
    }
     
    int MyLinkedList::Count()const
    {
    	node *temp;
    	int c=0;
    	for(temp=m_start; temp!=NULL; temp=temp->next)
    		c++;
    	return c;
    }

    Comment il faut que je m y prenne?

    Ce sont mes constructeurs /operateur= qui sont faux ou mon destructeur?

    Merci bcp !

  4. #4
    screetch
    Invité(e)
    Par défaut
    c'est la copie et l'operateur = qui sont faux
    quel est ton code de test ensuite ?

  5. #5
    Expert éminent
    Avatar de koala01
    Homme Profil pro
    aucun
    Inscrit en
    Octobre 2004
    Messages
    11 644
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Âge : 53
    Localisation : Belgique

    Informations professionnelles :
    Activité : aucun

    Informations forums :
    Inscription : Octobre 2004
    Messages : 11 644
    Par défaut
    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:
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    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
    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
    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
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    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
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    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
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    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
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    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
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    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
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    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
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    node* n = temp.m_start;
    temp.m_start = m_start;
    m_start = temp;
    ou de
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    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>
    A méditer: La solution la plus simple est toujours la moins compliquée
    Ce qui se conçoit bien s'énonce clairement, et les mots pour le dire vous viennent aisément. Nicolas Boileau
    Compiler Gcc sous windows avec MinGW
    Coder efficacement en C++ : dans les bacs le 17 février 2014
    mon tout nouveau blog

  6. #6
    Expert éminent
    Avatar de Médinoc
    Homme Profil pro
    Développeur informatique
    Inscrit en
    Septembre 2005
    Messages
    27 395
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Âge : 41
    Localisation : France

    Informations professionnelles :
    Activité : Développeur informatique
    Secteur : High Tech - Éditeur de logiciels

    Informations forums :
    Inscription : Septembre 2005
    Messages : 27 395
    Par défaut
    Si tu utilises la fonction std::swap() dans ton operator= sans la surcharger, tu auras un débordement de pile: swap() est libre d'utiliser l'opérateur =...

    Le mieux est de se faire une fonction membre void MyLinkedList::Swap(MyLinkedList&), et de l'appeler explicitement depuis ton operator=.

    Ensuite, tu es libre de surcharger la fonction swap() pour qu'elle appelle MyLinkedList::Swap() au lieu d'utiliser =.
    SVP, pas de questions techniques par MP. Surtout si je ne vous ai jamais parlé avant.

    "Aw, come on, who would be so stupid as to insert a cast to make an error go away without actually fixing the error?"
    Apparently everyone.
    -- Raymond Chen.
    Traduction obligatoire: "Oh, voyons, qui serait assez stupide pour mettre un cast pour faire disparaitre un message d'erreur sans vraiment corriger l'erreur?" - Apparemment, tout le monde. -- Raymond Chen.

Discussions similaires

  1. probleme avec copy constructor
    Par alamata dans le forum C++
    Réponses: 15
    Dernier message: 06/11/2014, 08h34
  2. vector et copy constructor
    Par r0d dans le forum Langage
    Réponses: 5
    Dernier message: 27/12/2012, 19h49
  3. SWAP IDIOM COPY CONSTRUCTOR
    Par guillaume07 dans le forum C++
    Réponses: 32
    Dernier message: 01/06/2010, 07h49
  4. [debutant]probleme avec le copy constructor
    Par Battosaiii dans le forum Débuter
    Réponses: 10
    Dernier message: 09/11/2005, 10h33
  5. Copy constructor ?
    Par Keyser dans le forum MFC
    Réponses: 4
    Dernier message: 17/02/2004, 15h33

Partager

Partager
  • Envoyer la discussion sur Viadeo
  • Envoyer la discussion sur Twitter
  • Envoyer la discussion sur Google
  • Envoyer la discussion sur Facebook
  • Envoyer la discussion sur Digg
  • Envoyer la discussion sur Delicious
  • Envoyer la discussion sur MySpace
  • Envoyer la discussion sur Yahoo