Une classe contenant un pointeur sur elle même

**scary** · 30/03/2010, 20h07

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 :

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
/***********************************************************************
 * 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 :

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
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 :

Sélectionner tout - Visualiser dans une fenêtre à part

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

**Arzar** · 30/03/2010, 21h11

Bonjour,

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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.

**bruno_pages** · 30/03/2010, 21h27

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

**scary** · 30/03/2010, 21h27

Justement même en spécifiant Papier* ça marche pas

**Arzar** · 30/03/2010, 21h34

Si, ça compile.

**bruno_pages** · 30/03/2010, 21h40

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

**scary** · 30/03/2010, 22h29

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 :

Sélectionner tout - Visualiser dans une fenêtre à part

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

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
Papier Papier::getPapierFinal(void) const
{
   return *this->papierFinal;
}

Encore merci

**bruno_pages** · 30/03/2010, 22h39

Envoyé par scary

Le fichier .h est dans mon premier post

je l'avais pas vu, bon, on va dire qu'il commence à être tard

**bruno_pages** · 30/03/2010, 22h51

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

**scary** · 30/03/2010, 23h08

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

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

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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;
}

**bruno_pages** · 30/03/2010, 23h21

et bien cette définition est exactement la définition par défaut avec tout ses désavantages

, il faut remplacer this->papierFinal = p.papierFinal par *papierFinal = *(p.papierFinal)

il est inutile d'écrire this->

**scary** · 30/03/2010, 23h38

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

. Et j'ai donc bien remplacer la ligne incriminée par:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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

Encore merci de la précision

**bruno_pages** · 30/03/2010, 23h53

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 ...

**mailaka** · 31/03/2010, 10h36

Envoyé par bruno_pages

*papierFinal = *(p.papierFinal)

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

**bruno_pages** · 31/03/2010, 10h52

Bonjour,

Envoyé par mailaka

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

oh oui, très bonne remarque

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

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

**scary** · 31/03/2010, 18h40

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 ?

**bruno_pages** · 31/03/2010, 19h01

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 :

Sélectionner tout - Visualiser dans une fenêtre à part

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

**scary** · 31/03/2010, 19h19

Ah en effet oui je n'avait mais alors vraiment pas pensé à ça du tout (en même temps je suis la pour apprendre

), j'ai donc modifié mon code en fonction ainsi que la redéfinition de l'opérateur= comme suit:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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 :

Sélectionner tout - Visualiser dans une fenêtre à part

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

Et le set:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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

**bruno_pages** · 31/03/2010, 19h27

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

**bruno_pages** · 31/03/2010, 19h39

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

, 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 ?

Une classe contenant un pointeur sur elle même

C++

Discussions similaires

Partager

Partager