Fuite mémoire - Map/Vector

Version imprimable

Bonjour à tous,

Je viens vous voir, vous les experts (enfin j'espère :aie: ) pour palier à mon problème de fuite mémoire.

Je travaille sur un petit programme de gestion de "forme".

J'ai donc 4 quatres classes.

Cercle - permettant d'instancier un Cercle avec son rayon et centre
Polygone - permettant d'instancier un Polygone par ces points contenu dans deux vector<double> x et y.

Forme, classe abstraite et parente de Cercle et Polygone

Usine permettant de stocker des formes (cercle, polygone) par le biais d'une map.

Mon main me permet d'instancier un cercle ou un polygone et de mettre une copie de celui-ci dans l'usine.

Pour un cercle, j'ai aucun soucis. Pour un polygone voici le code suivi :

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
 
string nomPolygone;
int nombrePoints;
vector<double> xPolygone, yPolygone;
double xTmp, yTmp;
Forme* tmp;
 
cout << "Identifiant de votre Polygone ?" << endl;	
cin >> nomPolygone;
cout << "" << endl;
cout << "Combien de points contiendra votre polygone ?" << endl;	
cin >> nombrePoints;
cout << "" << endl;
 
for(i=0;i<nombrePoints;i++)
{
	cout << "Enregistrement N " << i+1 << " :" << endl;
	cout << "--------------------" << endl;
 
	cout << "X = ";
	cin >> xTmp;						
	cout << "Y = ";
	cin >> yTmp;
	xPolygone.push_back(xTmp);
	yPolygone.push_back(yTmp);
}
tmp = new Polygone(xPolygone, yPolygone);
test = U.ajouterForme(nomPolygone,tmp);
 
if(test) cout << "Ajout réussi" << endl;
else cout << "Ajout non réussi - la clé existe déjà" << endl;
 
delete tmp;

On va donc dans ajouterForme (const string&, Forme*);

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
 
bool Usine::ajouterForme(const string &_identifiant, Forme* obj)
{
	//Insertion de l'identifiant et de l'objet dans une pair et ensuite dans le catalogue
	//Le pointeur de Forme ajouté dans l'usine correspond à une copie de celui passé en parametre
	if(obj==0) 
	{
		return 0;
	}	
	else if(!catalogue.count(_identifiant)) 
	{ 
		catalogue.insert(pair<string, Forme*>(_identifiant,obj->dupliquer())); 
		return 1; 
	}
	else
	{
		return 0;
	}
}

On appelle donc dupliquer() (de polygone par polymorphisme)

Code:

1
2
3
4
5
 
Forme* Polygone::dupliquer() const
{
	return new Polygone(*this);
}

qui appelle le constructeur de recopie de polygone

Code:

1
2
3
4
5
6
7
 
Polygone::Polygone(const Polygone& obj)
{
	x = obj.x;
	y = obj.y;
	cout << "CREATION D'UN POLYGONE PAR RECOPIE" << endl;
}

Et la dans le constructeur de recopie, c'est le drame ! En testant j'ai vu que si je commentais les deux lignes de code sur les vecteurs, j'avais aucune fuite, avec ...

valgrind me donne en rapport :

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
 
==2513== 
==2513== HEAP SUMMARY:
==2513==     in use at exit: 192 bytes in 6 blocks
==2513==   total heap usage: 34 allocs, 28 frees, 706 bytes allocated
==2513== 
==2513== 32 bytes in 1 blocks are definitely lost in loss record 1 of 6
==2513==    at 0x402641D: operator new(unsigned int) (vg_replace_malloc.c:255)
==2513==    by 0x805418F: __gnu_cxx::new_allocator<double>::allocate(unsigned int, void const*) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8053DF9: std::_Vector_base<double, std::allocator<double> >::_M_allocate(unsigned int) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x805707A: double* std::vector<double, std::allocator<double> >::_M_allocate_and_copy<__gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > > >(unsigned int, __gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > >, __gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > >) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8056E3F: std::vector<double, std::allocator<double> >::operator=(std::vector<double, std::allocator<double> > const&) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x80566C7: Polygone::Polygone(std::vector<double, std::allocator<double> >, std::vector<double, std::allocator<double> >) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8050D13: main (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513== 
==2513== 32 bytes in 1 blocks are definitely lost in loss record 2 of 6
==2513==    at 0x402641D: operator new(unsigned int) (vg_replace_malloc.c:255)
==2513==    by 0x805418F: __gnu_cxx::new_allocator<double>::allocate(unsigned int, void const*) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8053DF9: std::_Vector_base<double, std::allocator<double> >::_M_allocate(unsigned int) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x805707A: double* std::vector<double, std::allocator<double> >::_M_allocate_and_copy<__gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > > >(unsigned int, __gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > >, __gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > >) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8056E3F: std::vector<double, std::allocator<double> >::operator=(std::vector<double, std::allocator<double> > const&) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x80566DC: Polygone::Polygone(std::vector<double, std::allocator<double> >, std::vector<double, std::allocator<double> >) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8050D13: main (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513== 
==2513== 32 bytes in 1 blocks are definitely lost in loss record 3 of 6
==2513==    at 0x402641D: operator new(unsigned int) (vg_replace_malloc.c:255)
==2513==    by 0x805418F: __gnu_cxx::new_allocator<double>::allocate(unsigned int, void const*) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8053DF9: std::_Vector_base<double, std::allocator<double> >::_M_allocate(unsigned int) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x805707A: double* std::vector<double, std::allocator<double> >::_M_allocate_and_copy<__gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > > >(unsigned int, __gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > >, __gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > >) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8056E3F: std::vector<double, std::allocator<double> >::operator=(std::vector<double, std::allocator<double> > const&) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8056784: Polygone::Polygone(Polygone const&) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8056D1A: Polygone::dupliquer() const (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8054A9C: Usine::ajouterForme(std::string const&, Forme*) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8050DAB: main (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513== 
==2513== 32 bytes in 1 blocks are definitely lost in loss record 4 of 6
==2513==    at 0x402641D: operator new(unsigned int) (vg_replace_malloc.c:255)
==2513==    by 0x805418F: __gnu_cxx::new_allocator<double>::allocate(unsigned int, void const*) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8053DF9: std::_Vector_base<double, std::allocator<double> >::_M_allocate(unsigned int) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x805707A: double* std::vector<double, std::allocator<double> >::_M_allocate_and_copy<__gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > > >(unsigned int, __gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > >, __gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > >) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8056E3F: std::vector<double, std::allocator<double> >::operator=(std::vector<double, std::allocator<double> > const&) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x805679C: Polygone::Polygone(Polygone const&) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8056D1A: Polygone::dupliquer() const (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8054A9C: Usine::ajouterForme(std::string const&, Forme*) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8050DAB: main (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513== 
==2513== 32 bytes in 1 blocks are definitely lost in loss record 5 of 6
==2513==    at 0x402641D: operator new(unsigned int) (vg_replace_malloc.c:255)
==2513==    by 0x805418F: __gnu_cxx::new_allocator<double>::allocate(unsigned int, void const*) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8053DF9: std::_Vector_base<double, std::allocator<double> >::_M_allocate(unsigned int) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x805707A: double* std::vector<double, std::allocator<double> >::_M_allocate_and_copy<__gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > > >(unsigned int, __gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > >, __gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > >) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8056E3F: std::vector<double, std::allocator<double> >::operator=(std::vector<double, std::allocator<double> > const&) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8056784: Polygone::Polygone(Polygone const&) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8056D1A: Polygone::dupliquer() const (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8054C11: Usine::creerForme(std::string const&) const (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x805100D: main (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513== 
==2513== 32 bytes in 1 blocks are definitely lost in loss record 6 of 6
==2513==    at 0x402641D: operator new(unsigned int) (vg_replace_malloc.c:255)
==2513==    by 0x805418F: __gnu_cxx::new_allocator<double>::allocate(unsigned int, void const*) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8053DF9: std::_Vector_base<double, std::allocator<double> >::_M_allocate(unsigned int) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x805707A: double* std::vector<double, std::allocator<double> >::_M_allocate_and_copy<__gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > > >(unsigned int, __gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > >, __gnu_cxx::__normal_iterator<double const*, std::vector<double, std::allocator<double> > >) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8056E3F: std::vector<double, std::allocator<double> >::operator=(std::vector<double, std::allocator<double> > const&) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x805679C: Polygone::Polygone(Polygone const&) (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8056D1A: Polygone::dupliquer() const (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x8054C11: Usine::creerForme(std::string const&) const (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513==    by 0x805100D: main (in /home/jonathan/TPC++/TP6/V1.1/Formes)
==2513== 
==2513== LEAK SUMMARY:
==2513==    definitely lost: 192 bytes in 6 blocks
==2513==    indirectly lost: 0 bytes in 0 blocks
==2513==      possibly lost: 0 bytes in 0 blocks
==2513==    still reachable: 0 bytes in 0 blocks
==2513==         suppressed: 0 bytes in 0 blocks
==2513== 
==2513== For counts of detected and suppressed errors, rerun with: -v
==2513== ERROR SUMMARY: 6 errors from 6 contexts (suppressed: 17 from 6)

Une idée ? (ou plus ...) :D

24/11/2011, 17h39
Trademark

Salut,

C'est difficile de savoir vu qu'on a pas tout ton code. Mais à mon avis c'est parce que tu ne libères pas la mémoire des vecteurs dans le destructeur de Polygon ?

Vérifie partout que tu as déssalloué tout ce qu'il fallait et notamment les vecteurs dont tu parles.

En effet, je ne désalloue jamais les vector<double>

Code:

1
2
3
4
5
 
Polygone::~Polygone()
{
	cout << "DESTRUCTION D'UN POLYGONE" << endl;
}

Il me semble que c'est automatique non ?

J'ai essayé de faire des x.clear() et y.clear() dans polygone pour tester sans succes.

Polygone.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
 
//Constructeur par défaut qui permet de créer un Polygone en définissant ses paramètres
Polygone::Polygone(vector<double> _x, vector<double> _y)
{	
	x = _x;
	y = _y;
	cout << "CREATION D'UN POLYGONE" << endl;
}
 
//Constructeur de recopie qui permet de créer un Polygone à partir d'un autre Polygone
Polygone::Polygone(const Polygone& obj)
{
	x = obj.x;
	y = obj.y;
	cout << "CREATION D'UN POLYGONE PAR RECOPIE" << endl;
}
 
//Destructeur de Polygone, les "vector" se supprime automatiquement par le biais du destructeur de la classe vector
Polygone::~Polygone()
{
	cout << "DESTRUCTION D'UN POLYGONE" << endl;
}
 
//obtenirPerimetre permet de connaitre le périmètre du Polygone, le calcul est basé sur le principe que les points sont dans l'ordre (1,2,3, ...)
double Polygone::obtenirPerimetre()const
{
	double perimetre=0;	
 
	for(int i=0;i<x.size()-1;i++)
	{
		perimetre = perimetre + sqrt(pow(x.at(i+1)-x.at(i),2)+pow(y.at(i+1)-y.at(i),2));
	}
 
	return perimetre + sqrt(pow(x.at(x.size()-1)-x.at(0),2)+pow(y.at(y.size()-1)-y.at(0),2));
}
 
//deplacerForme permet de faire une translation pure à la Forme
void Polygone::deplacerForme(double _xDeplacement, double _yDeplacement)
{
	for(int i=0;i<x.size();i++)
	{
		x[i] = x[i] + _xDeplacement;
		y[i] = y[i] + _yDeplacement;
	}
}
 
void Polygone::afficher()const
{
 
	cout << "|-----------------------------|" << endl;
	cout << " Polygone    " << endl;
 
 
	for(int i=0;i<x.size();i++)
	{
		cout << " Point " << i+1 << ":  x=" << x.at(i) << " , y=" << y.at(i) << endl;
	}
	cout << "" << endl;
	cout << " Périmètre = " << obtenirPerimetre() << endl;
	cout << "" << endl;
	cout << "" << endl;
	cout << "" << endl;
	cout << "" << endl;
 
 
}
 
//dupliquer permet de retourner une copie du pointeur sur l'objet courant this.
Forme* Polygone::dupliquer() const
{
	Forme* tmp = new Polygone(this->x, this->y);
	return tmp;
}
 
#endif

Usine.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
 
//Constructeur par défaut
Usine::Usine()
{
	cout << "CREATION D'UNE USINE" << endl;
}
 
//Destructeur par défaut, celui-ci vide les formes de l'usine et supprime la map
Usine::~Usine()
{
	map<string,Forme*>::iterator it;
 
	for (it=catalogue.begin();it!=catalogue.end();it++)
	{
		delete (*it).second;
	}
	catalogue.clear();
	cout << "DESTRUCTION D'UNE USINE" << endl;
}	
 
//ajouterForme permet d'ajouter une forme dans l'usine en se basant sur une forme et son identifiant
bool Usine::ajouterForme(const string &_identifiant, Forme* obj)
{
	//Insertion de l'identifiant et de l'objet dans une pair et ensuite dans le catalogue
	//Le pointeur de Forme ajouté dans l'usine correspond à une copie de celui passé en parametre
	if(obj==0) 
	{
		return 0;
	}	
	else if(!catalogue.count(_identifiant)) 
	{ 
		catalogue.insert(pair<string, Forme*>(_identifiant,obj->dupliquer())); 
		return 1; 
	}
	else
	{
		return 0;
	}
}
 
//retirerForme permet de retirer une forme en supprimant le pointeur sur la forme stocké dans l'usine puis en supprimant la ligne dans la map.
bool Usine::retirerForme(const string &_identifiant)
{		
	map<string,Forme*>::iterator it;
 
	it = catalogue.find(_identifiant);
	if(it==catalogue.end()) return 0;
	else
	{
		delete (*it).second;
 
		//On efface une ligne de la map	
		catalogue.erase(it);
		return 1;
	}
}
 
//Cette méthode consiste à chercher et à dupliquer une forme dans une usine
Forme* Usine::creerForme(const string &_identifiant) const
{
	//On recupère une forme dans la map sur la base d'un Id et on le retourne
	//Déclaration d'un itérateur de map de type const permettant 
	map<string,Forme*>::const_iterator it;
	it = catalogue.find(_identifiant);
	return ((*it).second)->dupliquer();	
}
 
//obtenirCles permet de récupérer la liste des clés de l'usine en parcourant toutes l'usine (classe map).
vector<string> Usine::obtenirCles(void) const
{
	vector<string> vect;
 
	//Déclaration d'un itérateur de map de type const permettant 
	map<string,Forme*>::const_iterator it;
 
	//On lit chaque ligne de la map et on affiche sa clé
  	for(it=catalogue.begin() ; it != catalogue.end(); it++) 
	{	
		vect.push_back((*it).first); 
	}
	return vect;
}
 
 
#endif

24/11/2011, 17h54
screetch

c'est parce que tu ne libères jamais tes polygones, qui eux a leur tour ne libèreront jamais leurs vecteurs. Les vecteurs sont bien libérés automatqiuement pour peu que tu libère bien tes polygones
24/11/2011, 17h58
screetch

au vu du rapport de leaks je dirais met un mot clé "virtual" avant le destructeur de Forme =)

Il me semble que si pourtant, dans mon main j'ai bien fait le delete tmp

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
string nomPolygone;
int nombrePoints;
vector<double> xPolygone, yPolygone;
double xTmp, yTmp;
Forme* tmp;
 
cout << "Identifiant de votre Polygone ?" << endl;	
cin >> nomPolygone;
cout << "" << endl;
cout << "Combien de points contiendra votre polygone ?" << endl;	
cin >> nombrePoints;
cout << "" << endl;
 
for(i=0;i<nombrePoints;i++)
{
	cout << "Enregistrement N " << i+1 << " :" << endl;
	cout << "--------------------" << endl;
 
	cout << "X = ";
	cin >> xTmp;						
	cout << "Y = ";
	cin >> yTmp;
	xPolygone.push_back(xTmp);
	yPolygone.push_back(yTmp);
}
tmp = new Polygone(xPolygone, yPolygone);
test = U.ajouterForme(nomPolygone,tmp);
 
if(test) cout << "Ajout réussi" << endl;
else cout << "Ajout non réussi - la clé existe déjà" << endl;
 
delete tmp;

et dans Usine.cpp, mon destructeur détruit les pointeurs Forme*

Il manque des destructeurs ou selon toi ?

24/11/2011, 18h49
Theri

Citation:

Envoyé par screetch

au vu du rapport de leaks je dirais met un mot clé "virtual" avant le destructeur de Forme =)

MMmmmm comment te dire ...

Merci ! :mouarf:

Merci Trademark aussi pour le dérangement.

:ccool:

C'est quand même un peu frustrant ... :aie:

Salut,

Ce ne sont que des détails, mais il a son importance pour te permettre d'évoluer :aie:

As tu déjà entendu parler de SRP :question:

Non, ce n'est pas le sigle d'un quelconque parti politique, cette abréviation signifie "Single Responsability Principle", ou, si tu préfères en français "Principe de la Responsabilité Unique" :D.

Ce principe nous dit que, si une fonction, une classe ou, au final, n'importe quoi a plus d'une responsabilité, c'est très clairement que ce "n'importe quoi" a trop de responsabilités.

C'est très clairement le cas de ta fonction, parce que, si tu y regarde d'un peu plus près, elle a
une responsabilité d'input (interrogation) parce qu'elle demande l'identifiant, le nombre et les coordonnées des angles de ton polygone
une responsabilité de création de ton polygone
une responsabilité d'ajout du polygone à "autre chose"
une responsabilité d'output (affichage)
et meme une responsabilité de destruction
Ca fonctionne!!!

Mais, si un jour, tu souhaites passer à une bibliothèque graphique, par exemple, tu sera obligé de "casser" ton code "simplement" parce que l'input et l'output (pour ne citer que ces deux là :D) se feront différemment :aie:

De plus, il existe un idiome nommé RAII (Resource Acquisition Is Initialization, ou , si tu préfères en français L'Acquisition des Ressources Sert d'Initialisation) qu'il est largement préférable d'utiliser.

L'idée est de faire en sorte, quand on crée un objet, de lui donner d'office toutes les données qui lui permettront d'être utilisable directement avec les données qui nous intéressent.

Cet idiome est largement facilité par l'existence des... constructeurs, et par la possibilité d'en définir autant que l'on veut, pour autant que les arguments passés soient à chaque fois différents en type et/ou en nombre :D

En outre, on pourrait estimer que l'identifiant du polynome fait lui même partie du polygone (en fait, carrément de toute forme) et sert... de discriminant!

Je suis d'accord que chaque polygone peut etre discriminé par l'adresse mémoire à laquelle il se trouve, mais, d'un autre coté, le fait que tu aies décidé de les placer dans une map dont la clé est l'identifiant et la valeur le pointeur sur le polygone montre bien que tu voudras régulièrement le retrouver... au départ de son identifiant :aie:

Il semble donc "cohérent" de veiller à ne pas "perdre" cet identifiant une fois que tu as quitté le "doux cocon" de la map, ce qui pourrait arriver si tu décidais, par exemple, de remplir un tableau contenant les pointeurs vers tous les polygones ayant 37 cotés, parce qu'il est toujours envisageable que tu veuille afficher les identifiants de tous ces polygones, voir, que tu veuille par la suite retrouvé l'élément untel :D

Enfin, je présumes que la fonction dupliquer va... renvoyer un pointeur qui pointe sur... un nouveau polygone qui n'est que... la copie du polygone que la fonction a créé!

Il est un peu dommage de se payer deux créations d'objet et une destruction, alors qu'une seule création pourrait amplement suffire, non :question:

En effet, dés que tu décide d'utiliser new, tu deviens seul juge du moment où l'objet sera détruit.

cela apporte la difficulté de devoir décider du moment le plus opportun de cette destruction, mais, par contre, cela t'apporte énormément de souplesse par le fait que, une fois créé, tant que tu ne perdra pas l'adresse mémoire à laquelle se trouve ton objet (et tant que tu n'auras pas décidé de le détruire, bien sur :D) , tu peux le trimbaler où tu veux sans avoir à payer le prix d'une copie qui peut s'avérer couteuse ;)

Bon, maintenant que j'ai fait mes remarques, je vais t'indiquer comment les prendre en compte, autrement, ce ne serait pas sympa :D

Pour le problème de la responsabilité unique, il faudrait créer au minimum une fonction pour chaque responsabilité (input, création de l'objet, insertion et output, étant acquis que la responsabilité de la destruction de l'objet échoira à l'objet dont la responsabilité est de garder l'ensemble des formes sous la main :D), mais, idéalement (souviens toi : tu pourrais très bien décider de passer à une bibliothèque graphique :D ) à une hiérarchie d'objets particuliers.

Tu pourrais ainsi avoir (je prévois l'évolution "bibliothèque graphique" :D) quelque une hiérarchie de classe proche de
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 AbstractPolygonDataAsker { /* les constructeurs par défaut nous vont très bien * * le destructeur est virtuel pour assurer le polymorphisme de * manière correcte */ ~virtual AbstractPolygonDataAsker(); /* la fonction qui nous intéressera au plus haut point * * Comme elle doit renvoyer trois informations distinctes (parce * que je ne change pas ton algorithme pour la cause :D), * nous lui passons des références non constantes sur chacune des * information qui nous intéresse * * param[in,out] name identifiant à utiliser pour le polygone * param[in,out] xTab tableau contenant, l'ensemble des valeurs sur * l'axe des x * param[in,out] yTab tableau contenant l'ensemble des valeurs sur * l'axe des y * * Cette fonction est virtuelle pure pour forcer l'utilisateur à la redéfinir * pour chaque solution d'input envisagée */ virtual void askData(std::string & name, std::vector<double> &xTab, std::vector<double> & ytab) = 0; /* je donne une responsabilité supplémentaire, uniquement parce que * je n'ai pas envie de créer une classe suplémentaire... mais CE N'EST * PAS BIEN */ virtual void showResult(bool result) =0; }; /* la classe qui demande le tout en mode texte */ class TextDataPolygonDataAsker : public AbstractPolygonDataAsker { /* param[in,out] name identifiant à utiliser pour le polygone * param[in,out] xTab tableau contenant, l'ensemble des valeurs sur * l'axe des x * param[in,out] yTab tableau contenant l'ensemble des valeurs sur * l'axe des y */ virtual void askData(std::string & name, std::vector<double> &xTab, std::vector<double> & ytab) { int nombrePoints; cout << "Identifiant de votre Polygone ?" << endl; cin >> name; cout << "" << endl; cout << "Combien de points contiendra votre polygone ?" << endl; cin >> nombrePoints; cout << "" << endl; for(i=0;i<nombrePoints;i++) { double xTmp; double yTmp; cout << "Enregistrement N " << i+1 << " :" << endl; cout << "--------------------" << endl; cout << "X = "; cin >> xTmp; cout << "Y = "; cin >> yTmp; xTab.push_back(xTmp); yTab.push_back(yTmp); } } virtual void showResult(bool result) { if(result == true) std::cout<<"l'ajout a reussi"<<std::endl; else std::cout<<"l'ajout a echoue"<<std::endl; } }; /* exemple fictif d'une classe qui demande le tout avec une IHM */ class MyIHMDataPolygonDataAsker : public AbstractPolygonDataAsker { /* param[in,out] name identifiant à utiliser pour le polygone * param[in,out] xTab tableau contenant, l'ensemble des valeurs sur * l'axe des x * param[in,out] yTab tableau contenant l'ensemble des valeurs sur * l'axe des y */ virtual void askData(std::string & name, std::vector<double> &xTab, std::vector<double> & ytab) { MyPrettyDialog dialog; if(dialog.execute()) { name = dialog.polygonName(); xTab.push_back(dialog.xValues().begin(), dialog.xValues.end()); yTab.push_back(dialog.yValues().begin(), dialog.yValues.end()); } } virtual void showResult(bool result) { if(result == true) { ResultSuccessDialog dialog; dialog.show(); } else { ResultFailureDialog dialog; dialog.show(); } } };
(j'ai mis un max de commentaires pour aider à la compréhension, mais n'hésites pas à poser des questions s'il reste des zones d'ombres :D)

Nous aurions pris la remarque concernant l'identifiant en compte en rajoutant un membre "identifiant" (de type chaine de caractères) à Forme et en rajoutant un constructeur acceptant une chaine decaractères afin de l'initialiser :D

Et, bien sur, au niveau de Polygone, nous aurions rajouté un constructeur acceptant:
un identifiant sous la forme d'une chaine de caractères
un tableau contenant l'ensemble des coordonnée sur l'axe des x
un tableau contenant l'ensemble des coordonnées sur l'axe des y
et qui prendrait donc une forme proche de
Code:

1 2 3 4 5 Polygone::Polygone(std::string const & id, std::vector<double> const &tabX, std::vector<double> const & tabY):Form(id), /* je ne sais pas comment tu as géré tes tableaux de X et de Y :-P */ { }
Comme je t'ai dit que la création du polygone (en fait, de n'importe quelle forme :D ) était une responsabilité à part, et que je présumes que, de toutes manières, il existe (ou il existera) aussi des classes telles que Rectangle, Triangle ou Cercle (et d'autres encore :D), mais que toutes formes ne seront jamais que... des objets de type Forme, il est peut etre utile d'utilser ce que l'on appelle une fabrique...

L'idée de la fabrique est de se dire que, finalement, on se fout royalement de savoir quel est le type réel de l'objet dont on demande la création du moment que l'on obtienne un objet correctement créé sur base des informations que l'on transmettra...

Tout ce que nous avons à savoir, c'est que nous obtiendrons un pointeur sur un objet du type de base (à savoir Forme dans notre cas)

Dans le cas présent, elle pourrait ressembler à quelque chose comme (je la fait compréhensible, mais il faudrait sans doute faire autrement :D )
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 class FormFactory { public: Form * createCircle(std::string const & id, double x, double y, double diametre) { return new Cercle(id,x, y, diametre); } Form * createRectangle(std::string const & id, double xTopLeft, double yTopLeft, double xBottomRight, double yBottomRight) { return new Rectangle(id, xTopLeft, yTopLeft, xBottomRight, yBottomRight); } Form * createPolygon(std::string const & id, std::vector<double> const & xTab, std::vector<double> const & yTab) { return new Polygon(id,xTab, yTab); } /* pareil pour les autres types intéressants ;) */ };
Enfin, en ce qui concerne le cout des deux constructions et de la destruction lorsque tu demande un nouveau polygone, il suffit de virer le delete et de modifier la fonction ajouterForme pour qu'elle ressemble à
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 bool Usine::ajouterForme(Forme* form) { if(catalogue.find(form->id) != catalogue.end()) { /* l'identifiant existe déjà, on détruit l'objet et on renvoie false */ delete form; return false; } /* si on arrive ici, l'identifiant n'existe pas, on l'ajoute donc * au catalogue et on renvoie true */ catalogue.insert(std::make_pair(form->id(),form)); return true; }
Au final, ton algorithme pourrait ressembler à quelque chose comme (je considère ici que l'on a déjà déterminé qu'il s'agissait d'un polygone, et que l'on dispose d'un pointeur sur un objet dérivé de AbstractPolygonDataAsker ;)
Code:

1 2 3 4 5 6 7 8 std::string identifiant; std::vector<double> xTab; std::vector<double> yTab; asker->askData(identifiant, xTab,yTab); Factory factory; Form * poly = factory.createPolygon(id, xTab, yTab); bool test = U.ajoute(poly); asker->showResult(result);
Tu vas sans doute me demander quel est l'avantage de travailler de la sorte...

D'abord, cela te fait des fonctions beaucoup plus courtes : tu remarqueras que ma fonction la plus grande est la fonction void TextDataPolygonDataAsker :: askData, et qu'elle ne fait que royalement 23 lignes, prototye compris (j'aurais d'ailleurs encore pu la factoriser, mais bon :D ) Tout est donc beaucoup plus lisible, car il est difficile de maitriser la logique d'une fonction si elle s'étend sur plus d'une page affichable (à l'époque du DOS, par exemple, il était de bon ton de dire qu'une fonction ne pouvait pas dépasser les 25 lignes de 80 caractères car c'était tout ce qu'il était possible d'afficher sur un écran ;) )

Ensuite, chaque fonction a une responsabilité précise, ce qui fait qu'il devient beaucoup plus facile de traquer les bugs

Enfin, comme chaque responsabilité échoit à un objet bien particulier, tu laisses toutes les possibilités d'évolution ouvertes...

Tu décides de donner le choix entre Qt et openGl pour l'affichage :question:

Pas de problème : tu dérives une classe de AbstractPolygonDataAsker ;).

Tu veux rajouter le type particulier dodécagone :question:

Pas de problème : tu modifies ta factory, et tu adaptes les comportements qui l'appelle ;)

Pour ce qui n'était que des détails, j'ai encore une fois écrit un roman, mais j'espère que tout cela te permettra d'évoluer :D

25/11/2011, 18h18
Theri

Citation:

C'est très clairement le cas de ta fonction, parce que, si tu y regarde d'un peu plus près, elle a
une responsabilité d'input (interrogation) parce qu'elle demande l'identifiant, le nombre et les coordonnées des angles de ton polygone
une responsabilité de création de ton polygone
une responsabilité d'ajout du polygone à "autre chose"
une responsabilité d'output (affichage)
et meme une responsabilité de destruction

Par rapport à l'architecture globale du programme, en réalité je n'avais pas le choix étant sur un mini projet de c++ pour mes études, les prototypes des méthodes étaient dans le cahier des charges.

Cependant, tes conseils sont très précieux (si si je te l'assure :D). En effet, malgré le cahier des charges j'aurais pu découper les entrées/sorties/affichage/etc, ce qui finalement aurait été aussi rapide et simple ...

Au sujet de la fabrique, est-ce une structure que tu prends en général lors de classes abstraite ?

Un roman en effet, mais de bonne augure pour ma compréhension des architectures logiciels, c'est pas mon dernier projet (juste le premier).
25/11/2011, 18h42
koala01

Citation:

Envoyé par Theri

Par rapport à l'architecture globale du programme, en réalité je n'avais pas le choix étant sur un mini projet de c++ pour mes études, les prototypes des méthodes étaient dans le cahier des charges.

Je comprend bien, et c'est pour cela que j'ai non seulement attendu que la discussion soit marquée résolue mais aussi (et surtout) bien expliqué que c'était du détail ;

Citation:

Cependant, tes conseils sont très précieux (si si je te l'assure :D). En effet, malgré le cahier des charges j'aurais pu découper les entrées/sorties/affichage/etc, ce qui finalement aurait été aussi rapide et simple ...

Tout à fait :D

Citation:

Au sujet de la fabrique, est-ce une structure que tu prends en général lors de classes abstraite ?

Il faut toujours se méfier des généralités, parce qu'il y aura toujours une exception pour la confirmer, mais, oui...

C'est carrément ce que l'on appelle un Design Pattern : un "patron de développement" ;)

Mais cela a l'énorme avantage de limiter les dépendances des classes qui manipulent tes objets polymorphes par rapport à la hiérarchie elle meme...

Les classes qui manipulent les objets polymorphes peuvent alors se "contenter" de connaitre la classe de base, du moins, tant que cette connaissance lui suffit ;)

Citation:

Un roman en effet, mais de bonne augure pour ma compréhension des architectures logiciels, c'est pas mon dernier projet (juste le premier).

Le gros problème, c'est que, bien souvent, on aborde en cours l'architecture logicielle de manière tout à fait séparée (et souvent après) les langages de programmation...

Or, dans l'ordre logique qui mene d'une idée à une application complète, l'architecture logicielle intervient dés le départ, et, normalement, avant meme d'écrire la première ligne de code ;)