Discussion :

[TheShade]

Bonjour à tous !

Je me suis créé une classe Lien qui contient dans ses membres privés le pointeur suivant :

Code :

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

CPen* pen

Dans le constructeur de la classe, je fais ceci :

Code :

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

pen = new CPen(PS_SOLID,2,RGB(255,0,0));

Tout fonctionne correctement, et le programme s'execute en utilisant bien le CPen.

Mais lorsque j'ajoute ceci :

Code :

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

1
2
3
4
Lien::~Lien()
{
	delete pen;
}

le programme plante.
Pourquoi ?

[aoyou]

Tu as implémenté le constructeur de copie et l'opérateur d'affectation ?

Je dirais que ton objet lien doit être copié mais la copie pointe sur le même objet CPen. Quand tu détruis la copie et l'original, il essaie de détruire deux fois pen ce qui ne va pas.

Avec si peu de code, il est difficile de voir le problème...

[TheShade]

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
// Lien.h: interface for the Lien class.
//
//////////////////////////////////////////////////////////////////////
 
#if !defined(AFX_LIEN_H__ED9A577A_C350_4B44_B6F9_FFD6107B0A79__INCLUDED_)
#define AFX_LIEN_H__ED9A577A_C350_4B44_B6F9_FFD6107B0A79__INCLUDED_
 
#if _MSC_VER > 1000
#pragma once
#endif // _MSC_VER > 1000
 
#include "Lieu.h"
 
class Lien  
{
public:
	Lien();
	Lien(CString n, Lieu* d, Lieu* a);
	virtual ~Lien();
 
	void dessiner(CPaintDC *dc);
 
private:
	Lieu* depart;
	Lieu* arrivee;
	CString nom;
 
	CPen* pen;
 
};
 
#endif // !defined(AFX_LIEN_H__ED9A577A_C350_4B44_B6F9_FFD6107B0A79__INCLUDED_)

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
// Lien.cpp: implementation of the Lien class.
//
//////////////////////////////////////////////////////////////////////
 
#include "stdafx.h"
#include "psc.h"
#include "Lien.h"
 
#ifdef _DEBUG
#undef THIS_FILE
static char THIS_FILE[]=__FILE__;
#define new DEBUG_NEW
#endif
 
//////////////////////////////////////////////////////////////////////
// Construction/Destruction
//////////////////////////////////////////////////////////////////////
 
Lien::Lien()
{
 
}
 
Lien::Lien(CString n, Lieu* d, Lieu* a)
{
	nom = n;
	depart = d;
	arrivee = a;
 
	pen = new CPen(PS_SOLID,2,RGB(255,0,0));
}
 
Lien::~Lien()
{
	delete pen;
}
 
void Lien::dessiner(CPaintDC *dc)
{
	dc->SelectObject(pen);
	dc->MoveTo(depart->getx(),depart->gety());
	dc->LineTo(arrivee->getx(),arrivee->gety());
}

Je ne vois pas ou se situe la copie dont tu parles...

[Davidbrcz]

Toute classe gérant en interne des données dynamiques (aloué par new) doivent diposer d'un constructeur de recopie et d'un opperateur d'affectation.
De cette façon, elle respecterons la forme canonique d'une classe et éviterons les divers problèmes lors d'une recopie/affectation.

Pour plus d'info : Cf n'importe quel bon cours de C++.

[Biozic]

Je ne vois pas ou se situe la copie dont tu parles...

La copie dont il parle n'est pas dans le code des méthodes de la classe, mais dans le code qui utilise la classe.

Sinon pose plutôt ta question dans le forum MFC. Je n'ai jamais utilisé les MFC, mais je dirais à vue de nez que tu devrais simplement créer et détruire ton CPen dans la méthode qui l'utilise (dessiner). Ça évite d'avoir à gérer un pointeur en tant que champ de la classe.

[nicroman]

Rajoutes juste ce bout de code dans la déclaration de la classe Lien:

Code :

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

1
2
3
4
 
private:
    // disabled....
    Lien& operator=(const Lien& ) {return *this; }

Et tu verras vite si c'est bien une recopie...
De toute manière, je conseille toujours de mettre *systématiquement* (et explicitement) les constructeurs de copie et opérateurs de recopie dans le code... quitte à ce qu'ils soient privés / non-accessible. Ca évite toujours de mauvaises surprises.

Et je conseilles aussi, après le delete, de mettre à 0 le pointeur "pen"... juste au cas ou quelqu'un s'amuserait à réutiliser l'objet après sa destruction... (avec par exemple un appele à "Dessiner")

[Médinoc]

Pareil, mais sans contenu:

Code C++ :

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

1
2
3
private:
    // disabled....
    Lien& operator=(const Lien& );

[Pacorabanix]

Et je conseilles aussi, après le delete, de mettre à 0 le pointeur "pen"... juste au cas ou quelqu'un s'amuserait à réutiliser l'objet après sa destruction... (avec par exemple un appele à "Dessiner")

comment ce peut-il ? puisque pen est membre de la classe qui justement détruit pen dans son destructeur ? ?

[Médinoc]

Tu ne sais jamais ce qu'une mauvaise manip de pointeur peut faire.

[Pacorabanix]

Médinoc Tu ne sais jamais ce qu'une mauvaise manip de pointeur peut faire

( va vite éditer ses destructeurs )...

[coyotte507]

Erreur

Dans ton constructeur de lien normal, tu n'alloues pas de pointeur:

Code :

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

1
2
3
4
Lien::Lien()
{
 
}

C'est normal que quand tu fais un delete il plante.

[Médinoc]

Le problème n'est pas qu'il n'est pas alloué, mais qu'il n'est pas initialisé non plus: delete NULL est garanti ne rien faire.

[Floréal]

Je pense que dans ce cas là, tu pourrais utiliser un pointeur intelligeant du TR1. Si le TR1 n'est pas implémenté dans ta STL, tu peux utiliser ceux fournis par Boost. Ainsi tu t'affranchirais de l'obligation du delete puisque le pointeur intelligent le fait tout seul.
-> http://cpp.developpez.com/faq/cpp/?p...ers#DIVERS_tr1
-> http://boost.org/doc/html/boost_tr1/...boost_tr1.ptrs

[scr]

A mon avis le problème vient du fait que ton objet est encore utilisé lorsque tu cherche à le liberer.

Lorsque tu utilise la fonction:
dc->SelectObject(pen);

Il faut faire:

Code :

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

1
2
3
4
5
6
 
// utilisation du pen
oldObject = dc->SelectObject(pen);
 
// fin d'utilisation du pen
dc->SelectObject(oldObject);