Une classe contenant un pointeur sur elle même

Version imprimable

Voir 40 message(s) de cette discussion en une page

Bonsoir,

J'ai une classe Papier qui a comme attribut un pointeur sur un Papier. Seulement le compilateur me crie dessus et je ne comprend pas pourquoi :(

le .h

Code:

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
/***********************************************************************
 * Module:  Papier.h
 * Author:  Scary
 * Modified: mardi 30 mars 2010 13:15:22
 * Purpose: Declaration of the class Papier
 ***********************************************************************/
 
#ifndef PAPIER_H
#define PAPIER_H
 
#include "TypePapier.h"
#include "EtatPapier.h"
 
class Papier
{
	private:
		Papier *papierFinal;
		TypePapier typePapier;
		EtatPapier etatPapier;
 
	public:
	    Papier ();
	    Papier (TypePapier typePapier, EtatPapier etatPapier, Papier papierFinal);
	    Papier (const Papier& oldPapier);
	    virtual ~Papier();
	    virtual TypePapier getTypePapier (void) const;
	    virtual EtatPapier getEtatPapier (void) const;
	    virtual Papier getPapierFinal (void) const;
	    virtual void setPapierFinal (Papier newPapierFinal);
	    virtual void setTypePapier (TypePapier newTypePapier);
	    virtual void setEtatPapier (EtatPapier newEtatPapier);
};
 
#endif

le .cpp

Code:

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
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
/***********************************************************************
 * Module:  Papier.cpp
 * Author:  Scary
 * Modified: mardi 30 mars 2010 13:15:22
 * Purpose: Implementation of the class Papier
 ***********************************************************************/
 
#include "Papier.h"
 
////////////////////////////////////////////////////////////////////////
// Name:       Papier::Papier()
// Purpose:    Implementation of Papier::Papier()
// Return:     
////////////////////////////////////////////////////////////////////////
 
Papier::Papier()
{
}
 
////////////////////////////////////////////////////////////////////////
// Name:       Papier::Papier(TypePapier typePapier, EtatPapier etatPapier, Papier papierFinal)
// Purpose:    Implementation of Papier::Papier()
// Parameters:
// - typePapier
// - etatPapier
// - papierFinal
// Return:     
////////////////////////////////////////////////////////////////////////
 
Papier::Papier(TypePapier typePapier, EtatPapier etatPapier, Papier papierFinal)
{
   // TODO : implement
}
 
////////////////////////////////////////////////////////////////////////
// Name:       Papier::Papier(const Papier& oldPapier)
// Purpose:    Implementation of Papier::Papier()
// Parameters:
// - oldPapier
// Return:     
////////////////////////////////////////////////////////////////////////
 
Papier::Papier(const Papier& oldPapier)
{
   papierFinal = oldPapier.papierFinal;
}
 
////////////////////////////////////////////////////////////////////////
// Name:       Papier::~Papier()
// Purpose:    Implementation of Papier::~Papier()
// Return:     
////////////////////////////////////////////////////////////////////////
 
Papier::~Papier()
{
   // TODO : implement
}
 
////////////////////////////////////////////////////////////////////////
// Name:       Papier::getTypePapier() const
// Purpose:    Implementation of Papier::getTypePapier()
// Return:     TypePapier
////////////////////////////////////////////////////////////////////////
 
TypePapier Papier::getTypePapier(void) const
{
   // TODO : implement
}
 
////////////////////////////////////////////////////////////////////////
// Name:       Papier::getEtatPapier() const
// Purpose:    Implementation of Papier::getEtatPapier()
// Return:     EtatPapier
////////////////////////////////////////////////////////////////////////
 
EtatPapier Papier::getEtatPapier(void) const
{
   // TODO : implement
}
 
////////////////////////////////////////////////////////////////////////
// Name:       Papier::getPapierFinal() const
// Purpose:    Implementation of Papier::getPapierFinal()
// Return:     Papier
////////////////////////////////////////////////////////////////////////
 
Papier Papier::getPapierFinal(void) const
{
   return this->papierFinal;
}
 
////////////////////////////////////////////////////////////////////////
// Name:       Papier::setPapierFinal(Papier newPapierFinal)
// Purpose:    Implementation of Papier::setPapierFinal()
// Parameters:
// - newPapierFinal
// Return:     void
////////////////////////////////////////////////////////////////////////
 
void Papier::setPapierFinal(Papier newPapierFinal)
{
   papierFinal = newPapierFinal;
}
 
////////////////////////////////////////////////////////////////////////
// Name:       Papier::setTypePapier(TypePapier newTypePapier)
// Purpose:    Implementation of Papier::setTypePapier()
// Parameters:
// - newTypePapier
// Return:     void
////////////////////////////////////////////////////////////////////////
 
void Papier::setTypePapier(TypePapier newTypePapier)
{
   // TODO : implement
}
 
////////////////////////////////////////////////////////////////////////
// Name:       Papier::setEtatPapier(EtatPapier newEtatPapier)
// Purpose:    Implementation of Papier::setEtatPapier()
// Parameters:
// - newEtatPapier
// Return:     void
////////////////////////////////////////////////////////////////////////
 
void Papier::setEtatPapier(EtatPapier newEtatPapier)
{
   // TODO : implement
}

Et l'erreur du compilo:

Code:

1
2
3
4
5
Papier.cpp(89): error C2664: 'Papier::Papier(const Papier &)' : cannot convert parameter 1 from 'Papier *const ' to 'const Papier &'
1>          Reason: cannot convert from 'Papier *const ' to 'const Papier'
1>          No constructor could take the source type, or constructor overload resolution was ambiguous
1>Papier.cpp(102): error C2440: '=' : cannot convert from 'Papier' to 'Papier *'
1>          No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called

Si quelqu'un aurait une idée :( Merci d'avance :)

30/03/2010, 21h11
Arzar
Bonjour,
Code:

1 2 3 4 Papier Papier::getPapierFinal(void) const { return this->papierFinal; }
papierFinal est de type Papier*, alors que ta fonction renvoie un objet de type Papier.
30/03/2010, 21h27
bruno_pages

pour compléter la réponse précédente, le compilateur vous indique clairement votre erreur en vous disant qu'un Papier n'est pas un Papier*, dans votre intérêt apprenez à lire les messages ... ou ne faites plus jamais d'erreur de syntaxe, au choix :lol:
30/03/2010, 21h27
scary

Justement même en spécifiant Papier* ça marche pas :(
30/03/2010, 21h34
Arzar

Si, ça compile.
30/03/2010, 21h40
bruno_pages

alors c'est que vous avez modifier la définition de l'attribut papierFinal qui était un pointeur et qui ne l'est plus, sinon il y aurait d'autres erreurs. La prochaine fois merci de donner aussi le .h pour nous aider à vous aider ;)
30/03/2010, 22h29
scary
Le fichier .h est dans mon premier post :) sinon autant pour moi en effet cela marchait seulement j'éditais le mauvais fichier:? (quel nul)

Pour info voila les changements que j'ai effectué:
Code:

1 2 3 4 void Papier::setPapierFinal(Papier newPapierFinal) { *papierFinal = newPapierFinal; }
Code:

1 2 3 4 Papier Papier::getPapierFinal(void) const { return *this->papierFinal; }
Encore merci :)
30/03/2010, 22h39
bruno_pages

Citation:

Envoyé par scary

Le fichier .h est dans mon premier post :)

:oops: je l'avais pas vu, bon, on va dire qu'il commence à être tard :aie:
30/03/2010, 22h51
bruno_pages
je suppose que l'attribut papierFinal est initialisé dans les constructeurs ?

attention : vous n'avez pas défini l'operateur=, donc lors de papier1=papier2; :
- papier1.papierFinal est perdu (memory leak)
- papier1 et papier2 partagent le même papierFinal, un setPapierFinal sur l'un des deux modifiera aussi l'autre
pour éviter les recopies lors des passages de paramètre vous devriez utiliser des références constantes : void setPapierFinal (const Papier & newPapierFinal), par contre c'est inutile pour les autres cas comme TypePapier si c'est une énumération par exemple

Oui mais je n'avais pas encore finis ma classe lorsque j'ai posté:mrgreen:

D'ailleurs voici la redéfinition de l'opérateur =
Code:

1 2 3 4 5 6 7 8 9 10 11 Papier& Papier::operator=(const Papier& p) { if (this != &p) { this->papierFinal = p.papierFinal; this->etatPapier = p.etatPapier; this->typePapier = p.typePapier; } return *this; }

30/03/2010, 23h21
bruno_pages

et bien cette définition est exactement la définition par défaut avec tout ses désavantages :aie:, il faut remplacer this->papierFinal = p.papierFinal par *papierFinal = *(p.papierFinal)

il est inutile d'écrire this->
30/03/2010, 23h38
scary

Pour une lisibilité personnelle (et par réflexe aussi) je préfère écrire "this->", tout comme je préfère écrire "std::" plusieurs fois plutot que "using namespace std", je sais j'aime bien écrire:mouarf::aie:. Et j'ai donc bien remplacer la ligne incriminée par:

Code:

*(this->papierFinal) = *(p.papierFinal);

Encore merci de la précision ;)
30/03/2010, 23h53
bruno_pages

you welcome

pour rebondir sur le this->, mon point de vue c'est dans le code tout doit se justifier c.a.d. que si quelque chose est écrit c'est qu'il le fallait sinon cela marche pas/mal. Cela évite au lecteur de se demander pourquoi il y a ce truc alors qu'il n'y a pas de raison. Mais bon ... ;)
31/03/2010, 10h36
mailaka

Citation:

Envoyé par bruno_pages

*papierFinal = *(p.papierFinal)

Bonjour. Ne fait-on pas un appel recursif sur operator= ici ??
31/03/2010, 10h52
bruno_pages

Bonjour,

Citation:

Envoyé par mailaka

Ne fait-on pas un appel recursif sur operator= ici ??

oh oui, très bonne remarque :ccool:

et en fait il y a exactement le même problème au niveau du constructeur, j'aurais dut y penser :oops:

cela veut dire qu'il ne faut pas initialiser papierFinal dans le constructeur de Papier avec une instance (papierFinal = new Papier) mais avec un pointeur nul (papierFInal = 0), et bien-sur modifier tout le reste en conséquence
31/03/2010, 18h40
scary

C'est à dire ?, je vois pas ou ce trouve le problème... Si je crée un objet p de type Papier et que je veux accéder à l'attribut p.papierFinal cela provoquera une erreur si j'initialise avec le pointeur null. Non ?
31/03/2010, 19h01
bruno_pages
Citation:

Envoyé par scary

je vois pas ou ce trouve le problème

vous parlez duquel ? car il y en a deux :

1) dans le constructeur sans argument tel qu'on le voit dans le (vieux) code l'attribut papierFinal qui est un pointeur sur Papier n'est pas initialisé. Il ne peut en aucun cas être initialisé avec un new Papier car celui-ci chercherait aussi a initialisé son papierFinal avec un new Papier etc jusqu'à ce que la pile explose. Donc il faut initialiser papierFinal avec 0 (pointeur nul) dans le cas du constructeur sans argument

2) setPapierFinal fait une copie en profondeur, or il se peut que papierFinal soit un pointeur nul soit pour this soit pour l'argument du set, dans les deux cas cela va mal se passer

en fait je me demande si votre setPapierFinal ne devrait pas plutôt être :
Code:

1 2 3 4 5 6 void Papier::setPapierFinal(Papier * newPapierFinal) { if (papierFInal != 0) delete papierFinal; papierFinal = newPapierFinal; }
mais ce n'est pas moi qui peut savoir ce que setPapierFinal doit finalement faire

Même question dans le cas du get

Ah en effet oui je n'avait mais alors vraiment pas pensé à ça du tout (en même temps je suis la pour apprendre:mrgreen:), j'ai donc modifié mon code en fonction ainsi que la redéfinition de l'opérateur= comme suit:
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 Papier& Papier::operator=(const Papier& p) { if (this != &p) { Papier *papierTemp = new Papier (*p.papierFinal); delete this->papierFinal; this->papierFinal = papierTemp; this->etatPapier = p.etatPapier; this->typePapier = p.typePapier; } return *this; }
Et le constructeur par copie:
Code:

1 2 3 4 Papier::Papier (const Papier& oldPapier) { this->papierFinal = oldPapier.papierFinal; }
Et le set:
Code:

1 2 3 4 5 6 7 8 void Papier::setPapierFinal(const Papier& newPapierFinal) { if (this->papierFinal != 0) { delete papierFinal; } *(this->papierFinal) = newPapierFinal; }
Par contre pour le get je pense pas que cela soit un problème puisque rien n'empeche de renvoyer un pointeur null du moment qu'on le test ce qui est renvoyé par le get. Enfin je me base sur le C pour dire cela donc je pense pas que cela soit vraiment fiable:aie:

31/03/2010, 19h27
bruno_pages

dans operator= il est inutile d'utiliser un papierTemp, par contre il faut vérifier si papierFinal n'est pas 0 avant de le détruire et avant de le recopier

sinon j'ai participé à une très longue discussion cet après midi ici où il est proposé d'utiliser swap, même si je ne suis toujours pas d'accord avec cela je pense que cette discussion est malgré tout très instructive et que vous devriez prendre le temps de la lire ;)
31/03/2010, 19h39
bruno_pages

désolé mais j'avais pas vu la suite de la dernière réponse

Papier:: Papier (const Papier& oldPapier) n'a pas besoin d'être défini tel qu'il est là car il fait ce que le compilateur produira par défaut. Mais est-ce que c'est vraiment ce qui est voulu ? car papierFinal sera partagé, ce qui est catastrophique car il sera ensuite détruit plusieurs fois (une fois par destruction du conteneur)

le set est lui aussi catastrophique :calim2:, car *(this->papierFinal) = newPapierFinal; essaye soit d'écrire à partir de l'adresse 0 si papierFinal est nul, soit d'écrire dans le papierFInal qui est libéré à la ligne juste au dessus

on tourne en rond, que voulez-vous faire au niveau sémantique ?
que voulez-vous faire avec Papier ? que représente papierFInal ?

Voir 40 message(s) de cette discussion en une page