Discussion :

[b4u]

Bonjour,
je cherche à manipuler la copie d'un objet, grâce au code réduit suivant:

Code :

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

1
2
	CXmlFile f = _xml_file;
	f.SetTraversalMode(M_PRE_ORDER);

La méthode appelée reste bloquée dans une boucle infinie. Quand je manipule l'objet original:

Code :

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

1
2
	CXmlFile &f = _xml_file;
	f.SetTraversalMode(M_PRE_ORDER);

le message est envoyé correctement; si mes souvenirs sont bons, lorsqu'on initialise un objet au moment de sa déclaration, c'est le constructeur de copie qui est utilisé (et non l'opérateur =). Je pensais que celui par défaut suffirait pour cette classe dont voici la définition:

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
 
#ifndef	_CXMLFILE_H
#define	_CXMLFILE_H
 
#include <stack>
#include <queue>
#include "tinyxml.h"
 
 
enum
{
	M_PRE_ORDER,
	M_LEVEL_ORDER
};
 
typedef TiXmlNode	CXmlNode;
 
class	CXmlFile
{
public:
	CXmlFile() {}
	~CXmlFile() {}
 
public:
	CXmlNode		*Open(const char *file);
	CXmlNode		*NextNode();
	CXmlNode		*CurrentNode() {return _node;}
 
public:
	void			SetTraversalMode(const int mode);
 
private:
	void		init_node_stack();
	void		init_node_queue();
 
private:
	TiXmlDocument               _doc;
	CXmlNode                      *_node;
	std::stack<CXmlNode *>   _stack;
	std::queue<CXmlNode *>  _fifo;
	int                                 _mode;
 
public:
	static int              NodeDepth(const CXmlNode *node);
	static const char   *GetAttribute(const CXmlNode *node, const char *key);
};
 
 
#endif	// _CXMLFILE_H

J'y ai donc rajouté le constructeur de copie suivant:

Code :

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

CXmlFile(const CXmlFile &copy) : _doc(copy._doc), _node(copy._node), _stack(copy._stack), _fifo(copy._fifo), _mode(copy._mode) {}

Mais le problème de départ persiste (surement parce que le constructeur de copie généré par défaut doit être le meme que celui-ci)... j'avoue que c'est la première fois que je suis amené à créer un constructeur de copie à la mano. Quelqu'un pourrait m'aiguiller vers une solution?
Thx!

[Matthieu Brucher]

Pour être sûr, appelle explicitement le constructeur par copie - sans le '=', directement l'objet à copier entre parenthèses -

[xxiemeciel]

salut,

je trouve ce code ci etrange :

Code :

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

1
2
3
 
        CXmlFile &f = _xml_file;
	f.SetTraversalMode(M_PRE_ORDER);

pourquoi mettre une reference devant le f ? Sinon il faut que tu definisse l'operateur= le constructeur par copy sera appeler si tu fais ceci :

Code :

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

1
2
3
 
        CXmlFile f(monObjetACopier);
	f.SetTraversalMode(M_PRE_ORDER);

dans ton code tu instancie un objet avec le constructeur par defaut et tu l'assignes a f avec l'operateur=

[b4u]

Pour être sûr, appelle explicitement le constructeur par copie - sans le '=', directement l'objet à copier entre parenthèses -

Oui j'ai déja essayé ça, mais en vain :-(

salut,

je trouve ce code ci etrange :

Code :

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

1
2
3
 
 
CXmlFile &f = _xml_file; f.SetTraversalMode(M_PRE_ORDER);

pourquoi mettre une reference devant le f ?

C'est simplement pour faire contraste avec le code juste au dessus et montrer que si je manipule l'objet original, tout va bien. Ce n'est pas du code que j'utilise, juste un exemple (un peu maladroit, j'avoue).

Sinon il faut que tu definisse l'operateur= le constructeur par copy sera appeler si tu fais ceci :

Code :

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

1
2
3
 
 
CXmlFile f(monObjetACopier); f.SetTraversalMode(M_PRE_ORDER);

dans ton code tu instancie un objet avec le constructeur par defaut et tu l'assignes a f avec l'operateur=

Je pensais que:

Code :

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

CXmlFile f(toto);

et:

Code :

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

CXmlFile f = toto;

étaient équivalents, et que dans les 2 cas, c'est le constructeur de copie qui est appelé?
Quoiqu'il en soit, le problème persiste

[Matthieu Brucher]

Dans ce cas, tu as une autre erreur à un autre endroit.

[Charlemagne]

Je suis pas sûr que ce soit ici le problème.
Mais Il n'est pas possible de copier les classes du genre 'fstream', car il n'y a pas de constructeur de copie pour éviter d'accéder au fichier de 2 manières différentes. C'est peut-être le cas de ton _xml_file.

[b4u]

ben imaginons j'ai 2 objets '_xml_file' et 'f' de type CXmlFile et j'appelle la méthode DoSomething() sur l'un puis sur l'autre. Si cette méthode s'éxecute normalement pour '_xml_file' mais plante pour 'f', c'est bien que ces 2 objets ne se trouvaient pas dans le meme état lors de l'appel a DoSomething()?
Mettons maintenant que 'f' est une copie de '_xml_file' au moment de l'appel, et l'appel ne bloque qu'avec l'objet 'f'... on peut en déduire le probleme vient de la copie de '_xml_file' dans 'f', non?

Mais Il n'est pas possible de copier les classes du genre 'fstream', car il n'y a pas de constructeur de copie pour éviter d'accéder au fichier de 2 manières différentes. C'est peut-être le cas de ton _xml_file.

La copie se fait, je n'ai pas d'erreur de compilation. Sinon il y a la définition du type de l'objet _xml_file dans mon premier post, peut-etre que ca pourrait vous aider...
merci encore :-)

[Matthieu Brucher]

operator= n'est pas défini, donc il prendra celui par défaut, il y a peut-être un bug avec cette méthode ?

[xxiemeciel]

En general il faut ce mefier des operateurs par defaut et du constructeur par defaut surtout si on a des membres qui ont besoin d'etre initialiser.

de plus dans le cas des pointeurs les operateur par defaut ne font pas de copies je crois ce qui peut poser des problemes.

[Jean-Marc.Bourguet]

Quand on écrit:

Code :

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

Classe c = c1;

il faut que Classe::operator= soit accessible et le compilateur peut au choix générer une construction par défaut suivit d'un appel à l'opérateur d'assignement ou une construction par copie.

Quand on ne déclare pas le constructeur de copie ou l'opérateur d'assignement, ils sont déclarés et définis inline par défaut, en utilisant les constructeurs de copies et les opérateurs d'assignement des membres. Si ceux-ci ne sont pas visibles, on a une erreur à la compilation.

Donc le constructeur de copie que tu donnes est équivalent au constructeur qui serait généré. Le problème probable est que dés qu'on a des pointeurs, on veut rarement (mais ça arrive, un test est de regarder ce que fait le destructeur avec ces pointeurs, s'il fait un delete dessus, c'est certain qu'on n'est pas dans un cas rare) copier les pointeurs mais dupliquer aussi les objets pointés ou du moins faire quelque chose d'autre.

Une règle simple (mais qui a des exceptions comme toute les règles simples), c'est que si tu as un destructeur qui fait autre chose que ce que fait le destructeur par défaut, tu dois aussi déclarer un constructeur de copie et un opérateur d'assignement (tu peux ne pas les définir et les déclarer privés, ce qui est la manière idiomatique d'indiquer que c'est une classe qui ne doit pas être copiée, il y a une manière en train de devenir idiomatique dans certains cercles qui est d'hériter de boost:non_copiable -- je ne suis pas sûr du nom de la classe).

La première chose que je fais quand je vois une classe qui a des membres générés par le compilateur (constructeur par défaut, constructeur de copie, assignement, destructeur), c'est de vérifier si la version générée convient bien à ce que fait la classe. Ma manière d'indiquer que cette vérification a été faite, c'est de mettre en commentaire la signature de ces membres.

La solution à ton probléme est soit d'utiliser une référence comme tu l'as fait -- mais je suppose que ça ne correspond pas à ton design sinon tu ne poserais pas la question -- soit d'implémenter correctement le constructeur de copie et l'opérateur d'assignement.

[Luc Hermitte]

Quand on écrit:

Code :

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

Classe c = c1;

il faut que Classe::operator= soit accessible et le compilateur peut au choix générer une construction par défaut suivit d'un appel à l'opérateur d'assignement ou une construction par copie.

Euh. Cela va à l'encontre des souvenirs que j'ai de mes dernières lectures ainsi que de mes observations.
Jusqu'à présent, j'ai toujours vu que "T v2 = v1;" appelait le constructeur de copie, et que cela ne compilait pas si le constructeur était déclaré explicite.
Je sens qu'il va falloir que je fasse une petite recherche dans la norme moi.

boost:non_copiable -- je ne suis pas sûr du nom de la classe).

boost::noncopyable

PS: Jean-Marc a fait un bon résumé des problématiques qui se posent ici.

[JolyLoic]

C'est aussi contraire à ce que j'ai en tête...

An object of class type can be initialized with a parenthesized expressionlist, where the expressionlist is construed as an argument list for a constructor that is called to initialize the object. Alternatively, a single assignmentexpression can be specified as an initializer using the = form of initialization. Either directinitialization semantics or copyinitialization semantics apply; see 8.5. [Example:

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
class complex {
// ...
public:
complex();
complex(double);
complex(double,double);
// ...
};
complex sqrt(complex,complex);
complex a(1); // initialize by a call of complex(double)
complex b = a; // initialize by a copy of a
complex c = complex(1,2); // construct complex(1,2)
// using complex(double,double)
// copy it into c
complex d = sqrt(b,c); // call sqrt(complex,complex) and copy the result into d
complex e; // initialize by a call of complex()
complex f = 3; // construct complex(3) using complex(double)
// copy it into f
complex g = { 1, 2 }; // error; constructor is required

—end example]

[Jean-Marc.Bourguet]

Je l'ai encore faite Chaque fois que j'aborde ce sujet, je me plante sur ce point...

[b4u]

ok merci pour toutes ces précisions sur la construction de copie

Code :

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

1
2
3
4
5
6
private:
	TiXmlDocument               _doc;
	CXmlNode                      *_node;
	std::stack<CXmlNode *>   _stack;
	std::queue<CXmlNode *>  _fifo;
	int                                 _mode;

Je pense que c'est la copie de l'objet de type TiXmlDocument, la source du problème (le reste étant une copie d'objets standards, de pointeurs + 1 entier) ... malheureusement ce n'est pas moi qui est écrit la classe TiXmlDocument :/

[b4u]

Finalement j'ai repensé autrement mon interface et le problème a disparu.
Comme d'hab merci tout le monde!

[Laurent Gomila]

Pense à cliquer sur