Discussion :

[oieretxe]

Bonjour,

je débute depuis peu en C++ et j'ai un soucis avec un exercice que je dois faire.
Cet exercice porte sur l'héritage, les classes virtuelles pures... Je pense avoir cerné l'essentiel mais j'ai un soucis avec des constructeurs avec paramètres par héritage. En effet, j'ai créé une classe CFormes dans laquelle je stocke des FormesGeometrique, qui peuvent être des carrés, des cercles ou des rectangle. La première partie appelée "création automatique et aléatoire" marche sans problème, le soucis vient de la seconde partie "création manuel" car il semblerait que les objets carré, rectangle ou cercle que je créés soit tous considérés comme des objets de la classe CFormesGeometrique. Je n'arrive vraiment pas à comprendre d'où vient mon erreur...

Je vous ai mis l'ensemble des fichier .c et .h en pièce jointe, si quelqu'un aurait le temps de s'y pencher dessus ça m'aiderait beaucoup.

PS : je suis débutant en C++, tous conseils et remarques sont bonne à prendre :-)

[therwald]

Mais encore? Pour qu'on te conseille il va falloir être plus précis: à QUOI vois tu que "tous les objets sont considérés comme des CFormeGeometrique"?

[oieretxe]

Quand j'exécute mon main, toute la première partie se passe bien. Je créée des objets CCarré, CCercle et CRectangle par des constructeur sans paramètre, puis je stocke ces classes filles de CFormegeometrique dans un tableau de pointeur d'un objet de la classe CFormes.

Par contre, lors de la seconde partie, je créée sans problème des cercles, carrés et rectangles qui sont stockés dans un tableau de pointeurs d'un nouveau objet de la classe CFormes. Par contre quand je veux affiché le contenu de ce tableau via cout <<, j'ai une erreur me disant que je fais appel à une fonction virtuelle pure...

Ce que je ne comprend vraiment pas c'est pourquoi l'utilisation sans paramètre n'engendre aucunes erreurs, alors que quand je passe par des constructeur avec paramètres ça plante...

J'espère avoir été plus clair, merci d'y avoir jeté un coup d'oeil!

[therwald]

Cela n'a rien à voir avec les constructeurs en eux-mêmes.
Tu enregistres les adresses d'objets locaux. Or comme tu les crée dans un switch, une fois sorti du switch après le break l'adresse que tu as enregistrée est devenu invalide, car tu es sorti du scope: l'objet est mort, la pile a continué sa vie, et tu es dans l'UB (comportement indéfini) complet...donc ton code fait n'importe quoi car ton pointeur référence une adresse invalide. Comme l'adresse est dans la pile, que tu as le droit d'adresser, tu n'as pas de segfault.
Il faut faire très attention à la durée de vie des objets, on ne recopie pas à la légère l'adresse d'une référence

[koala01]

Salut,

Il y a tellement de choses à dire au sujet de ton code, que je me demande par quoi commencer...


Je crois que je vais commencer par ta classe CFormes, parce que là, franchement... il y a de la place pour le progres

La première chose que l'on pourrait en dire, histoire d'en finir une bonne fois pour toutes, c'est que les tableaux de taille dynamique en C++, c'est la classe vector, disponible dans l'espace de noms std au travers de l'inclusion du fichier d'en-tête <vector>.

Mais bon, comme je me doutes que ton prof risque de t'avoir demandé de gérer tes tableaux à la main (l'idiot ), une fois que c'est dit, on va quand même regarder un tout petit peu ton code

Le constructeur, tout d'abord:

A priori, on préférera toujours utiliser les listes d'initialisation au niveau des constructeurs, mais ce n'est, très clairement, pas le problème principal.

Le problème, c'est que tu ne sembles pas avoir conscience de ce que fait réellement new dans sa version new UnType[taille]: il demande l'allocation d'un espace mémoire de taille suffisante pour pouvoir y placer taille éléments.

Autrement dit, quand ton constructeur présente le code

Code :

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

1
2
3
4
5
6
 
CFormes::CFormes()
{
    taille = 0 ;
    cfg = new CFormeGeometrique * [taille] ;
}

cela revient à demander

affecte à cfg (au pointeur de pointeurs) l'adresse mémoire qui correspond à un espace suffisant pour représenter 0 éléments de type "pointeur sur CFormeGeometrique".

D'après la norme, ce genre de demande (qui consiste à demander l'allocation dynamique d'un espace mémoire de taille 0) résulte en un undefined behaviour, ou, si tu préfères, en un comportement indéfini.

Autrement dit, tu as peut être lancé des missiles sur les états unis sans même le savoir

Si tu ne sais pas quelle taille donner à ton tableau, initialises le à NULL (nullptr en C++11), pour clairement indiquer qu'il s'agit d'une adresse invalide, en attente d'initialisation

Intéressons nous donc à CFormes::add_forme, parce que là aussi, il y a la place pour le progrès.

Le code de cette fonction est, pour rappel, mais attention, ca pique aux yeux

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
void CFormes::add_forme(CFormeGeometrique & ccfg)
{
	if(taille == 0)
	{
		cfg[taille] = &ccfg;
		taille++;
	}
	else
	{
		CFormeGeometrique ** cfg2 = new CFormeGeometrique* [taille];
 
		for(int i=0 ; i<taille ; i++)
		{
			cfg2[i] = cfg[i];
		}
		cfg = new CFormeGeometrique*[taille] ;
		for(int i=0 ; i<taille ; i++)
		{
			cfg[i] = cfg2[i];
		}
		delete [] cfg2;
		cfg[taille] = &ccfg;
		taille++ ;
	}
}

Et réfléchissons un peu à ce qu'il fait faire.

Les lignes

Code :

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

1
2
3
4
5
6
 
if(taille == 0)
{
    cfg[taille] = &ccfg;
    taille++;
}

posent déjà un très gros problème.

Elles disent "si la taille est égale à 0, (autrement dit : si c'est la première fois que l'on ajoute une forme), alors, on affecte à cfg[0] (autrement dit, à ce qui devrait correspondre au premier élément de cfg) l'adresse de ccfg, puis incrémente la valeur de taille (autrement dit, fait passer la taille à 1.

Sauf que... Comme je te l'ai fait remarquer plus haut, cfg n'est absolument pas correctement initialisé! A priori, il devrait valoir NULL, et tu ne pourrais donc pas affecter l'adresse de ccfg (au sujet duquel il y a énormément à dire d'ailleurs) au premier élément de cfg, vu que, par défaut, c'est un pointeur invalide

Le contenu du else n'est pas mal dans son genre non plus, d'ailleurs. Regardons pourquoi (par facilité, je numérote les différentes étapes ) :

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
else
{
    CFormeGeometrique ** cfg2 = new CFormeGeometrique* [taille]; // 1
 
    for(int i=0 ; i<taille ; i++) //
    {                             //
        cfg2[i] = cfg[i];         //    2
    }                            //
    cfg = new CFormeGeometrique*[taille] ; // 3
    for(int i=0 ; i<taille ; i++) //
    {                             //
        cfg[i] = cfg2[i];         //    4
    }                            //
    delete [] cfg2; // 5
    cfg[taille] = &ccfg;  // 6
    taille++ ;   // 7
}

En 1, tu demande l'alllocation d'un espace mémoire suffisant pour représenter l'ensemble des pointeurs qui existent déjà (cfg2).
En 2, tu copies l'ensemble des pointeurs qui existent déjà vers l'espace mémoire dont tu viens de demander l'allocation dynamique.

Jusque là, c'est à peu près cohérent, dans le sens où, grace à l'incrémentation de taille qui est faite dans le if et à la fin du else, si taille vaut 1, tu as effectivement 1 élément à copier et qu'il est accessible au départ de l'index 0.

Il y a certaines choses à en dire, mais bon... je vais les garder pour un peu plus tard

En 3, tu demande d'affecter à cfg l'adresse mémoire à laquelle commence un espace suffisant pour représenter taille pointeurs, et là, ca commence à coincer:

D'abord parce que si taille vaut 1 à ce moment là, c'est que tu t'apprêtes à rajouter le deuxième pointeur, et qu'il vont donc se trouver "un peu à l'étroit" dans un espace qui n'est prévu que pour en contenir qu'un : si taille vaut 1 et que tu t'apprêtes donc à ajouter un deuxième pointeur, l'espace mémoire dont tu as besoin correspond à l'espace suffisant pour y placer... deux pointeurs. Ca parait logique, non

Ensuite, parce que, si taille vaut, mettons 2, c'est que tu as déjà demandé au moins une fois d'allouer de la mémoire pour y placer quelque chose, et que la seule variable qui fait référence à l'adresse mémoire en question, c'est cfg.

Si tu affectes directement une autre valeur à cfg, sans avoir veillé à ce que l'adresse mémoire à laquelle il fait référence a bien été libérée, tu perds irrémédiablement la référence vers l'adresse mémoire d'origine.

Autrement dit, tu ne pourras plus jamais récupérer cette adresse mémoire et faire en sorte de libérer correctement la mémoire qui a été allouée.

C'est ce que l'on appelle une fuite mémoire.

Ce comportement est très problématique, parce que, à terme (si il se reproduit assez souvent), c'est la stabilité de tout le système d'exploitation qui sera remise en cause, avec comme conséquence le fait de n'avoir pas d'autre choix que... de rebooter la machine

En 4, tu recopies les pointeurs que tu avais sauvegardés et en 5 tu libères la mémoire que tu avais allouée pour la sauvegarde, ce qui est une bonne chose. Mais bon, ne te réjouis pas trop vite, je reviendrai sur ce point.

En 6, tu affectes au pointeur dont l'index est aille (autrement dit 1 si taille vaut 1) l'adresse mémoire de ccfg.

Sauf que, si taille vaut 1, le seul index disponible est... : pour toute taille, l'intervalle d'indexes disponibles correspond à [0,taille[ (autrement dit, de l'index 0 inclus à taille -1 inclus, taille étant exclu).

l'adresse de ccfg va donc systématiquement se retrouver un cran trop loin par rapport à l'espace mémoire qui a été réservé pour y placer les pointeurs.

Et cela a de grandes chances de provoquer des catastrophes, parce que:
Si l'adresse mémoire correspondant à ce "cran trop loin" est déjà utilisé par "autre chose", le fait d'y écrire l'adresse mémoire de ccfg va modifier une valeur qui n'aurait jamais du l'être.
Si ce n'est pas le cas, cette adresse mémoire peut, à tout moment, être allouée "à autre chose" par le système d'exploitation (étant donné que, pour lui, elle est libre!), et que cette "autre chose" risque de modifier la valeur qui s'y trouve, ce qui fait que, lorsque tu essayeras d'accéder à la forme géométrique en question, ben, tu seras "dans la chouchroute"

Enfin, en 7, tu incrémentes la taille. Je n'ai rien à dire sur le sujet.

Maintenant, je voudrais revenir sur les points que j'ai laissés en suspend.

Tu auras remarqué que

1. tu fais deux allocations de mémoire (respectivement pour cfg2 (en1) et pour cfg (en 3) )
2. tu fais deux copies de contenus ( en 2 et en 4)
3. tu fais une libération de mémoire (en 5), mais que tu en oublies une
4. que tu alloues de manière systématique l'espace mémoire pour un élément trop peu.

Ne crois tu pas qu'il serait beaucoup plus logique de suivre un raisonnement proche de

1. j'alloue un espace suffisant pour représenter tous les pointeurs, y compris pour celui que je vais rajouter (cfg2)(autrement dit, pour taille +1 éléments)
2. Je copie les pointeurs existant à leur place dans cet espace mémoire
3. je libère la mémoire qui était précédemment allouée pour représenter ces pointeurs (cfg) car je n'en ai plus besoin
4. j'assigne la copie (cfg2) à l'original (cfg)
5. j'ajoute l'adresse du nouvel élément à sa place dans le tableau
6. j'incrémente la taille pour qu'elle corresponde au nombre effectif d'éléments dans le tableau

Je manque malheureusement de temps pour t'expliquer le problème au niveau de la fonction main (d'autres s'en chargeront sans doute rapidement) aujourd'hui, mais, si personne ne s'est dévoué d'ici vendredi, je t'expliquerai tous les problèmes auxquels tu est confronté

Saches que je n'ai pas parlé du destructeur de CFormes car, pour comprendre ce qu'il y aurait à en dire, il faut d'abord avoir compris ce qui ne va pas dans main.

Mais ca, c'est une autre histoire

[oieretxe]

Merci beaucoup de l'aide que tu m'as apporté. J'ai effectué les modifications suite à ton message, j'espère ne rien avoir oublié.

Constructeur de CForme :

Code :

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

1
2
3
4
5
6
CFormes::CFormes()
{
    cout<<"Constructeur par default de CFormes\n";
    taille = 0 ;
    cfg = new CFormeGeometrique * [1] ; // Je m'étais en effet complètement trompé en mettant taille entre crochets...
}

Maintenant pour ce qui est de add_forme :

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
void CFormes::add_forme(CFormeGeometrique & ccfg)
{
	cout << "\tAjout de la forme geometrique\n\n";
	if(taille == 0)
	{
		cfg[taille] = &ccfg;
		taille++;
	}
	else
	{
		CFormeGeometrique ** cfg2 = new CFormeGeometrique* [taille];
 
		for(int i=0 ; i<taille ; i++)
		{
			cfg2[i] = cfg[i];
		}
		delete []cfg; // 1
		cfg = new CFormeGeometrique*[taille+1] ; // 2
		for(int i=0 ; i<taille ; i++)
		{
			cfg[i] = cfg2[i];
		}
		delete [] cfg2;
		cfg[taille] = &ccfg;
		taille++ ;
	}
}

1 -> je libère l'espace mémoire
2 -> je redimensionne cfg pour avoir suffisamment de place pour les pointeurs déjà présents + le nouveau pointeur.

Malgré ces modifications, mon problème persiste... Ma première partie du main marche correctement, la seconde dans laquelle je créée des objets avec des constructeurs à arguments ne marche pas...
therwald : j'ai aussi pensé que le problème venait de là mais en passant par des if() else if() ..., j'ai toujours la même erreur...

[therwald]

therwald : j'ai aussi pensé que le problème venait de là mais en passant par des if() else if() ..., j'ai toujours la même erreur...

que tu utilises des if/else ou un switch ne change rien: l'intérieur des accolades est un scope, quand tu en sors ton objet est détruit car il est local à l'intérieur du bloc. Du coup l'adresse que tu as enregistrée dans ton tableau de pointeurs de Cformes est invalide (ce qu'on appelle un dangling pointer) et donc ça n'a aucune raison de fonctionner.
Pour que ça marche, il faut que dans ton add_forme tu fasses une COPIE de ton objet en utilisant new et le constructeur de copie.
Par ailleurs, dans le destructeur de ton objet forme, il faut qu'avant de libérer ton tableau tu itères sur tous les éléments pour libérer chaque forme avec un delete. Ensuite, tu libères ton tableau de pointeurs.