problème avec le constructeur de copie

problème avec le constructeur de copie - C++ - Programmation

Marsh Posté le 01-02-2011 à 16:01:11    

Bonjour,
 
J'ai défini un classe Personnage. J'essaie de copier le Personnage goliath vers le Personnage david comme suit :
 

Code :
  1. int main()
  2. {
  3. Personnage goliath("Epée aiguisée", 20);
  4. Personnage david = goliath;
  5. ...
  6. }


 
Voici les constructeurs que j'ai crée dans la classe Personnage :

Code :
  1. Personnage::Personnage() : m_vie(100), m_mana(100)
  2. {
  3. m_arme = new Arme();
  4. }
  5. Personnage::Personnage(string nomArme, int degatsArme) : m_vie(100), m_mana(100)
  6. {
  7. m_arme = new Arme(nomArme, degatsArme);
  8. }


 
Le compilateur me dit alors :
 
Undefined reference to 'Personnage::Personage(Personnage const& )
 
Ma question :
-- Le compilateur ne crée - t - il pas automatiquement le constructeur de copie ?
-- le problème serait - il ailleurs ?
 
Merci d'avance pour votre aide.


Message édité par razuki le 01-02-2011 à 17:51:52
Reply

Marsh Posté le 01-02-2011 à 16:01:11   

Reply

Marsh Posté le 01-02-2011 à 16:32:49    

Dans le code que tu as poste, il y a deux fois le meme constructeur. J'imagine que tu as un constructeur par defaut et un constructeur prenant deux parametres.
 
Le compilateur devrait creer un constructeur de recopie par defaut ... Mais dans ton cas, il ne sera pas bon car, les pointeurs m_arme "pointront" vers la meme "arme" (le constructeur par defaut se contentant de copier coller les valeurs des membres de l'objet copie). Il faut donc que tu definisses un constructeur de recopie faisant une copie correct des pointeurs (donc au moins de m_arme).
 

Reply

Marsh Posté le 01-02-2011 à 17:34:49    

Le problème est résolu. En fait j'ai déclaré le constructeur de copie mais je ne l'ai pas défini ...
Sinon un autre problème toujours lié au constructeur de copie ( et donc tu as parlé un peu  dans ta réponse ) :
 
-- j'ai deux Personnage david et goliath qui ont un pointeur arme pointant vers le meme objet arme (voir main ci-dessus).
-- j'ai détruit david ( d'abord son arme, puis lui meme ), puis j'ai détruit goliath et ceux en fin de main( )
 

Code :
  1. goliath.~Personnage();
  2. david.~Personnage();


 
Ma question :
est-ce normal que le compilateur ne rale pas, que le programme ne plante pas ... ?
En effet quand je détruit goliath l'arme disparait avec lui. Quand je tente de détruire david, le compilateur est censé cherche son arme ... NON ?
 
Merci d'avance pour votre aide.  

Message cité 1 fois
Reply

Marsh Posté le 01-02-2011 à 17:43:14    

razuki a écrit :

Code :
  1. goliath.~Personnage();
  2. david.~Personnage();



 
 :heink:  
 
Comment supprimes-tu des objets, toi ? "delete", ca te parle ?
 
Poste un peu le code de ton destructeur, pour voir.

Reply

Marsh Posté le 01-02-2011 à 17:50:26    

Code :
  1. Personnage::~Personnage()
  2. {
  3. delete m_arme;
  4. }


Message édité par razuki le 01-02-2011 à 17:50:39
Reply

Marsh Posté le 01-02-2011 à 18:26:14    

Pour l'instant, je ne vois pas d'où vient ce prodige, mais j'ai deux remarques à faire :
 
1) On n'appelle JAMAIS un destructeur directement. Il faut toujours passer par "delete". Je parierai que si tu remplace tes deux horribles appels aux destructeurs par des "delete" en bonne et due forme, ton programme va se planter comme tu t'y attends.
 
2) L'opérateur "delete" fait deux choses : il invoque le destructeur de l'objet ET il libère la mémoire occupée par cet objet.  
 
Si dans ta fonction ~Personnage() tu avais appelé m_arme->~Arme() au lieu de faire un "delete m_arme", l'espace mémoire occupé par m_arme n'aurait pas été libéré, donc ca aurrait pu expliquer pourquoi ca ne posait pas de souci de "détruire" cette ressource commune.


Message édité par shaoyin le 01-02-2011 à 18:26:48
Reply

Marsh Posté le 01-02-2011 à 21:20:17    

Dans ton constructeur par copie, ce n'est pas l'adresse de "arme" que tu dois dupliquer, mais l'objet "arme". Chaque objet Personnage doit avoir sa propre "arme".  
 

Message cité 1 fois
Reply

Marsh Posté le 02-02-2011 à 00:42:40    

J'ai vu quelque part que le compilateur fait appel implicitement au destructeur. C'est ce que shaoyin voulait me dire ppeut etre dans son 1). J'ai aussi essayé de ne pas mettre  les deux lignes ci-dessous au début, mais le compilateur ne rale pas non plus. Je m'attendais à ce qu'il rale.  
1. goliath.~Personnage();
2. david.~Personnage();

Reply

Marsh Posté le 02-02-2011 à 00:54:48    

Si j'ai bien compris :
1) le compilateur (?) détruit implicitement systématiquement david et goliath avant de terminer le programme.
2) Je n'ai donc pas besoin de faire appel au destructeur.
 
david utilise une arme A, goliath utilise la meme arme A. Les deux pointent sur le meme objet A.
-- le compilateur (?) détruit implicitement systematiquement david ET donc son arme => OK
-- le compilateur (?) détruit implicitement systematiquement goliath MAIS il ne trouve pas l'arme de celui-ci car l'arme a été détruite avec david ...
 
Pourquoi le compilateur ne rale pas ?

Message cité 1 fois
Reply

Marsh Posté le 02-02-2011 à 01:13:10    

razuki a écrit :

Si j'ai bien compris :
1) le compilateur (?) détruit implicitement systématiquement david et goliath avant de terminer le programme.
2) Je n'ai donc pas besoin de faire appel au destructeur.


 
1) Oui chaque variable est detruite lorsqu'elle sort du scope:

Code :
  1. int main() {
  2.   Personnage a;
  3.   {
  4.       Personnage b;
  5.   } // destruction de "b"
  6. } // destruction de "a"


 
2) on n'appelle jamais un destructeur manuellement. Par contre, tu DOIS utiliser "delete" pour liberer memoire allouee par "new", celui-ci se chargera d'appeler le destructeur (ou les destructeurs en cas d'heritage)
 
Ensuite le compilateur en tant que tel ne te dira jamais rien sur ce genre de probleme (ca serait trop facile ;)), c'est a l'execution que ca plantera (memoire corrompue). Ce code devrait planter:

Code :
  1. int main(void)
  2. {
  3.     Personnage a("epee", 20);
  4.     Personnage b = a;
  5.     return 0;
  6. }


Message édité par mr simon le 02-02-2011 à 01:13:27
Reply

Marsh Posté le 02-02-2011 à 01:13:10   

Reply

Marsh Posté le 02-02-2011 à 09:56:19    

xilebo a écrit :

Dans ton constructeur par copie, ce n'est pas l'adresse de "arme" que tu dois dupliquer, mais l'objet "arme". Chaque objet Personnage doit avoir sa propre "arme".  
 


Pas forcément, s'il y a beaucoup de personnages et une banque de données d'arme, et que chaque personnage ne modifie pas sa propre arme il aurait meilleur compte à implémenter une classe de banque d'armes qui instancie les armes utilisées sur demande, et à fournir à chaque personnage une fonction donnant accès à une référence vers une arme constante :

Code :
  1. const Arme& getArme();

Reply

Marsh Posté le 02-02-2011 à 09:56:56    

Dans tous les cas, les pointeurs bruts c'est la porte ouverte à toutes les fenêtres, c'est pas la la solution ici.

Message cité 1 fois
Reply

Marsh Posté le 02-02-2011 à 10:50:23    

hephaestos a écrit :

Dans tous les cas, les pointeurs bruts c'est la porte ouverte à toutes les fenêtres, c'est pas la la solution ici.


:lol:


---------------
Si la vérité est découverte par quelqu'un d'autre,elle perd toujours un peu d'attrait
Reply

Marsh Posté le 02-02-2011 à 11:59:09    

Bonjour,  
Je débute en C++ et j'aimerais provoquer des erreurs justement à l'exécution pour voir comment ca fonctionne.
 
 
Voici mon code Personnage.h :

Code :
  1. class Personnage
  2. {
  3.     public:
  5. Personnage();
  6. Personnage(std::string nomArme, int degatsArme);
  7. Personnage(const Personnage &personnageACopier);
  8.     void recevoirDegats(int nbDegats);
  9.     void attaquer(Personnage &cible);
  10.     void boirePotionDeVie(int quantitePotion);
  11.     void changerArme(std::string nomNouvelleArme, int degatsNouvelleArme);
  12.     bool estVivant();
  13.     void afficherEtat();
  15.     ~Personnage();
  18.     private:
  20.     int m_vie;
  21.     int m_mana;
  22. Arme* m_arme;
  23.     //std::string m_nomArme; // Pas de using namespace std, donc il faut mettre std:: devant string.
  24.     //int m_degatsArme;
  27. };


Voici mon code Personnage.cpp :

Code :
  1. #include <iostream>
  2. #include <string>
  3. #include "Personnage.h"
  5. using namespace std;
  7. //Personnage::Personnage() : m_vie(100), m_mana(100), m_nomArme("Epée rouillée" ), m_degatsArme(10) {}
  8. Personnage::Personnage() : m_vie(100), m_mana(100)
  9. {
  10. m_arme = new Arme();
  11. }
  12. //Personnage::Personnage(string nomArme, int degatsArme) : m_vie(100), m_mana(100), m_arme(nomArme, degatsArme) {}
  13. Personnage::Personnage(string nomArme, int degatsArme) : m_vie(100), m_mana(100)
  14. {
  15. m_arme = new Arme(nomArme, degatsArme);
  16. }
  18. Personnage::Personnage(const Personnage &personnageACopier)
  19. {
  20.     m_vie = personnageACopier.m_vie;
  21.     m_mana = personnageACopier.m_mana;
  22.     m_arme = personnageACopier.m_arme;
  23.     //m_arme = new Arme(*(personnageACopier.m_arme));
  25. }
  27. Personnage::~Personnage()
  28. {
  29. delete m_arme;
  30. }
  31. void Personnage::recevoirDegats(int nbDegats)
  32.     {
  33.  m_vie -= nbDegats;
  34.  if (m_vie < 0)
  35.   m_vie = 0;
  36.     }
  37.  void Personnage::attaquer(Personnage &cible)
  38.  {
  39.   cible.recevoirDegats(m_arme->getDegats());
  40.  }
  42.     void Personnage::boirePotionDeVie(int quantitePotion)
  43.     {
  44.      m_vie += quantitePotion;
  45.      if (m_vie > 100)
  46.       m_vie = 100;
  47.     }
  49.     void Personnage::changerArme(std::string nomNouvelleArme, int degatsNouvelleArme)
  50.     {
  51.      m_arme->changer(nomNouvelleArme, degatsNouvelleArme);
  53.     }
  55.     bool Personnage::estVivant()
  56.     {
  57.      if (m_vie > 0)
  58.       return true;
  59.      else
  60.    return false;
  61.     }
  63.     void Personnage::afficherEtat()
  64.     {
  65.         cout << "Vie : " << m_vie << endl;
  66.         cout << "Mana : " << m_mana << endl;
  67.         m_arme->afficher();
  68.     }


 
Mon main :
 

Code :
  1. int main()
  2. {
  3.     // Création des personnages
  4.     //Personnage david, goliath("Epée aiguisée", 20);
  5. Personnage goliath("Epée aiguisée", 20);
  6. Personnage david = goliath;
  7.     // Au combat !
  8.     goliath.attaquer(david);
  9.     david.boirePotionDeVie(20);
  10.     goliath.attaquer(david);
  11.     david.attaquer(goliath);
  12.     goliath.changerArme("Double hache tranchante vénéneuse de la mort", 40);
  13.     goliath.attaquer(david);
  15.     // Temps mort ! Voyons voir la vie de chacun...
  16.     cout << "David" << endl;
  17.     david.afficherEtat();
  18.     cout << endl << "Goliath" << endl;
  19.     goliath.afficherEtat();
  20.     //goliath.~Personnage();
  21.     //david.~Personnage();
  23.     return 0;
  24. }


 
le résultat après execution :

Code :
  1. David
  2. Vie : 40
  3. Mana : 100
  4. Arme : Epée aiguisée (Dégâts : 20)
  6. Goliath
  7. Vie : 80
  8. Mana : 100
  9. Arme : Double hache tranchante vénéneuse de la mort (Dégâts : 40)


 
Donc là le programme ne plante pas, contrairement à quoi je m'attendais.


Message édité par razuki le 02-02-2011 à 12:16:42
Reply

Marsh Posté le 02-02-2011 à 13:53:24    

Hum... d'un côté, ton programme est en train de se terminer lorsque se passe la double destruction du champ "m_arme". Alors le plantage n'est peut-être pas vraiment mis en évidence.
 
Tu peux essayer de mettre tout le contenu de ta fonction "main" (sauf le 'return 0;') dans un bloc, et de faire d'autres choses après ce bloc (des écritures sur std::cout, par exemple).  

Reply

Marsh Posté le 02-02-2011 à 14:06:15    

C'est bizarre que tu te retrouves avec deux armes différentes alors qu'effectivement ton constructeur de copie garde le même pointeur...

Reply

Marsh Posté le 02-02-2011 à 14:08:06    

J'avais pas fait gaffe à ca...


Message édité par shaoyin le 02-02-2011 à 14:09:31
Reply

Marsh Posté le 02-02-2011 à 14:08:59    

Oui, et il devrait aussi changer l'arme de David quand il change l'arme de Goliath, et ce n'est pas le cas.


Message édité par hephaestos le 02-02-2011 à 14:09:05
Reply

Marsh Posté le 02-02-2011 à 14:14:51    

razuki, tu peux poster le code de ta classe "Arme", stp ?

Reply

Marsh Posté le 02-02-2011 à 14:34:37    

shaoyin > j'ai regroupé toutes les instructions dans main( ) ( sauf return 0 ) dans un bloc. Puis j'ai rajouté un cout << "test"  à l'extérieur du bloc. Effectivement il n'affiche pas "test" quand David et Goliath possèdent le meme objet arme. Par contre, "test" s'affiche bien quand les deux personnages possède deux objets armes différents.
 
Hephaestos> moi non plus j'ai pas remarqué qu'il n'a pas changé l'arme de David ...
 
Je vais voir plutard et je vous tiendrai au courant.

Reply

Marsh Posté le 02-02-2011 à 15:04:09    

Je serais bien curieux de voir le code de la classe Arme, car avec le code que tu as poste, je ne vois pas comment David peut garder son arme (je ne peux meme pas imaginer comment realiser cela avec le code du constructeur de recopie que tu as).
 
Es tu sur que tu as poster le bon code pour Personnage, car si tu as :

Code :
  1. m_arme = new Arme(*(personnageACopier.m_arme));


(qui est en commentaire), ca marchera.
 
PS1: Pour les arguments de type std::string, utiliser une reference constante (i.e const std::string& ) est presque toujours une bonne idee!
PS2: Rendre les methodes constantes (par exemple afficherEtat()) est aussi une bonne pratique

Reply

Marsh Posté le 02-02-2011 à 15:14:42    

.h
 

Code :
  1. class Arme
  2. {
  3.     public:
  5.     Arme();
  6.     Arme(std::string nom, int degats);
  7.     void changer(std::string nom, int degats);
  8.     void afficher();
  9.     int getDegats() const;
  10.     //string setNomArme() const;
  12.     private:
  14.     std::string m_nom;
  15.     int m_degats;
  16. };
  18. #endif


 
.cpp  
 

Code :
  1. Arme::Arme() : m_nom("Epée rouillée" ), m_degats(10)
  2. {
  4. }
  6. Arme::Arme(string nom, int degats) : m_nom(nom), m_degats(degats)
  7. {
  9. }
  11. void Arme::changer(string nom, int degats)
  12. {
  13.     m_nom = nom;
  14.     m_degats = degats;
  15. }
  17. void Arme::afficher()
  18. {
  19.     cout << "Arme : " << m_nom << " (Dégâts : " << m_degats << " )" << endl;
  20. }
  22. int Arme::getDegats() const
  23. {
  24.     return m_degats;
  25. }


Reply

Marsh Posté le 02-02-2011 à 15:36:35    

Rien de bien extraordinaire la dedans.
 
Es-tu sur de ta sortie? car ca n'a pas de sens pour moi ...

Reply

Marsh Posté le 02-02-2011 à 15:48:14    

Moi aussi, je sèche complètement sur ce problème...

Reply

Marsh Posté le    

Reply

Sujets relatifs:

Leave a Replay

Make sure you enter the(*)required information where indicate.HTML code is not allowed