Discussion :

[Krishna]

Bonjour!
Dans ma classe CGnomeapp, j'ai besoin de créer deux pointeurs vers deux autres classes CEchiquier et CFeuille. J'ai donc ecrit:

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
#include <gtkmm.h>
#include <iostream>
 
class CEchiquier;
class CFeuille;
 
class CGnomeapp : public Gtk::Window
{
    public:
    //constructeur et destructeur
    CGnomeapp();
    virtual ~CGnomeapp();
    protected:
    CEchiquier* m_pechiquier;
    CFeuille* m_pfeuille;
};

et dans le .cc:

Code :

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

1
2
3
4
5
6
7
8
9
10
11
#include "CGnomeapp.h"
#include "CEchiquier.h"
#include "CFeuille.h"
 
CGnomeapp::CGnomeapp():m_pechiquier(new CEchiquier),m_pfeuille(new CFeuille),m_lfenetre(600),m_hfenetre(600)
{....}
CGnomeapp::~CGnomeapp()
{
    delete m_pfeuille;
    delete m_pechiquier;
}

lorsque je ferme le programme j'obtiens:

*** glibc detected *** /home/...: free(): invalid pointer: 0x00000000006c7658 ***
======= Backtrace: =========
/lib64/libc.so.6[0x2b1f76e69f9a]
/lib64/libc.so.6(cfree+0x8c)[0x2b1f76e6dc1c]
/home/...[0x40e4fb]
/home/...[0x41ef15]
/home/...[0x421aa3]
/lib64/libc.so.6(__libc_start_main+0xf4)[0x2b1f76e18ae4]
/home/...(_ZN4Glib6ObjectD1Ev+0x61)[0x408089]
======= Memory map: ========
etc...

Par contre, si je suprime les delete du destructeur tout va bien....
Pourtant, si j'ai bien compris le cours de C++ du site il est nécéssaire de faire suivre mes deux "new" de la liste d'initialisation par des "delete" non?

[Feriaman]

Salut,

Tu as parfaitement raison de faire un "delete" sur des pointeurs que tu as alloué avec "new".

Mais es-tu sur de ne le faire qu'une seule fois ? Je suppose que tu ne nous a pas livré tout ton code.
Es-tu sur que ton destructeur de CGnomeapp n'est appelé qu'une seule fois ?
Est-ce que les destructeurs de CEchiquier et de CFeuille ne font pas des delete qui, seraient responsables de l'erreur ?
As-tu d'autres delete dans ton code ?




Note : un jour, peut-être, tu utiliseras des structures dédiées pour gérer l'allocation et la désallocation de mémoire ceci à ta place, et ne pas avoir à le faire à la main.
C'est une deuxième étape une fois que tu auras bien compris comment ça marche.
Cela te permet d'éviter ce genre d'erreur.

[loicounet]

Si ton pointeur n'est pas initialisé, lorsque ton objet est détruit le delete va détruire quelque chose qui n'est pas alloué!

Soit dans ton constructeur tu fais le new et dans ton destructeur tu fait le delete.
Soit tu fais le new dans ton programme et tu delete toi même(en ne mettant pas de delete dans ton destructeur).

bonne chance

[Krishna]

J'ai regarder dans mon code et le seul endroit où j'utilise d'autres "new" se trouve justement dans une des deux classes concernées:

Code :

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

1
2
3
4
5
6
class CEchiquier : public Gtk::Table
{
/..../
    protected:
    CCase* m_ptab[9][9];
};

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
#include "CEchiquier.h"
 
//constructeur et destructeur
CEchiquier::CEchiquier():Table(10,10,true)
{
    //init du tableau de CCase
    m_ptab[b][1]=new CCase(b,1,"cb","cb");m_ptab[d][1]=new CCas/.../etc...
 
    show_all_children();
 
}
CEchiquier::~CEchiquier()
{
    delete[] m_ptab;
}

Feriaman, pourrais-tu m'en dire un peu plus sur ces structures dédiées?(un lien peu-être).

En tous cas si je comprend bien delete[] m_ptab est appelé avant le delete m_pechiquier mais je ne vois pas quelle contrainte peut exister entre ces deux instructions.

[Feriaman]

1) ordre des destructeurs

En tous cas si je comprend bien delete[] m_ptab est appelé avant le delete m_pechiquier mais je ne vois pas quelle contrainte peut exister entre ces deux instructions.

Ce n'est pas complètement exact.
En fait, c'est delete m_pechiquier appelle le destructeur de CEchiquier.

Si tu avais écrit :

Code :

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

1
2
3
4
5
6
7
CGnomeapp::~CGnomeapp()
{
    delete m_pfeuille;
    std::cout << "ceci ";
    delete m_pechiquier;
    std::cout << "phrase.";
}

et

Code :

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

1
2
3
4
5
6
CEchiquier::~CEchiquier()
{
    std::cout << "est ";
    delete[] m_ptab;
    std::cout << "une ";
}

Tu aurais pu lire sur la sortie standard :

Ceci est une phrase

2) façon "class" de gérer ses ressources

Feriaman, pourrais-tu m'en dire un peu plus sur ces structures dédiées?(un lien peu-être).

Oui avec plaisir.
Mais il faut bien comprendre que le type de structure que tu vas utiliser dépend de ce dont tu as besoin.
Il faut aussi comprendre que l'utilisation de ces structures est plus compliquée au début, et ne peut pas remplacer la compréhension du fonctionnement de new et delete.

Tu as un point d'entrée vers les pointeurs intelligents de boost ici
Tu as un point d'entrée vers la Flyweight Factory (que je n'ai personnellement pas encore utilisé, parce que je l'ai découverte récemment, mais qui a l'air séduisant et qui m'a l'air d'être un ressourceManager particulier) ici

De façon générale, tu as toutes les réponses à cette question dans cet excellent article



3) Les tableaux
Tu gagnerais beaucoup à remplacer :

Code :

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

CCase* m_ptab[9][9];

Par un

Code :

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

std::vector<CCase> m_ptab;

Mais ce n'est pas une opération triviale, cela suppose que tu apprennes à te servir d'un vector.

Tu as un point d'entrée ici.

Encore une fois, cela ne remplace pas la compréhension du fonctionnement des tableaux.


4) L'erreur dans ton code
l'erreur dans ton code est là :

Code :

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

1
2
3
4
5
6
7
8
9
10
 
class CEchiquier
{
    CCase* m_ptab[9][9];
}
...
CEchiquier::~CEchiquier()
{
    delete[] m_ptab;
}

l'objet m_ptab, tel que tu l'as déclaré est de type

Code :

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

CCase* m_ptab[9][9];

, c'est à dire

tableau de pointeurs de CCase

C'est à dire qu'il existe 82 zones mémoires qui "correspondent" à ce tableau.

- La première est une zone mémoire qui contient les adresses des 81 autres, il s'agit de ton tableau lui même, son adresse est :

Code :

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

&m_ptab

- Les 81 sont des zones mémoires qui contiennent des CCases, leur adresses sont de la forme

Code :

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

m_ptab[4][6]

Quand tu écris :

Code :

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

delete[] m_ptab;

Tu crois que tu supprimes les 81 zones mémoires correspondant aux cases en question, mais en réalité tu essaie de supprimer la première, ce qui crée ton erreur.

[Krishna]

J'ai modifié le destructeur de CEchiquier comme suit:

Code :

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

1
2
3
4
5
6
7
8
CEchiquier::~CEchiquier()
{
    int i,j;
    for(i=0;i<10;i++){
        for(j=0;j<10;j++){
            delete m_ptab[i][j];
        }}
}

Et je n'ai plus d'erreur...apparemment...Dis moi si je fais une autre boulette!
-Pour les <vector> il m'est arrivé de les utiliser dans un programme précédent. Mais comme il s'agit là des cases d'un échiquier, je trouve qu'un tableau est plus "lisible".
Mais en regardant l'organigramme des conteneurs je me dis que <map> a l'air pas mal du tout.
-Merci aussi pour les liens vers boost & co.

[Feriaman]

Dis moi si je fais une autre boulette!

Quelques pistes :
- Je ne reviens pas sur l'utilisation du vector.
- On évite en général les "nombre magique" dans le code.

Aussi, au lieu d'écrire :

Code :

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

CCase* m_ptab[9][9];

et

Code :

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

1
2
3
4
for(i=0;i<10;i++){
        for(j=0;j<10;j++){
            delete m_ptab[i][j];
        }}

,
tu devrais écrire au minimum :

Code :

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

#define TAILLE_JEU_ECHEC 9

ou

Code :

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

const int TAILLE_JEU_ECHEC=9;

Code :

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

CCase* m_ptab[TAILLE_JEU_ECHEC][TAILLE_JEU_ECHEC];

et

Code :

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

1
2
3
4
for(i=0;i<=TAILLE_JEU_ECHEC;i++){
        for(j=0;j<=TAILLE_JEU_ECHEC;j++){
            delete m_ptab[i][j];
        }}

.

Cela ne coute rien, est c'est TRES pratique si un jour tu veux changer cela. Certes tu pourrais rétorquer que tu ne changeras JAMAIS cela, mais il se trouve qu'en programmation, à chaque fois qu'on dit "JAMAIS", on se trompe.


- Utiliser des chaines de caractères pour représenter tes pièces n'est pas une bonne chose ("cb" représente le cavalier blanc ?)

Quand on fait un modèle de donnée, on essaie de faire en sorte qu'il ne soit pas possible d'y faire entrer des erreurs.

Les très mauvais programmes ne se donnent pas cette peine. Dans ton exemple, s'ils trouvent une pièce sur l'échiquier qui s'appelle "zt" par exemple, leur programme a un comportement indéterminé.

Les mauvais programmes font cela en utilisant tout un tas de tests du genre

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
void bougePiece(const std::string& piece)
{
   if (piece == "cb")
   {
      ...
   }
   else if (piece == "tb")
   {
      ...
   }
   ...
   else
   {
       afficheLErreurEtSort();
   }
}

Les bons programmes n'ont pas besoin de faire cela, car leur modèle de donnée fait en sorte que ce n'est tout simplement pas possible.

Dans ton cas, on pourrait par exemple conseiller quelque chose comme ceci :

Code :

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

1
2
3
4
5
6
enum PiecesPossibles
{
   CavalierNoir;
   TourNoire;
   ...
};

Ou des choses encore plus compliquées, mais plus sures.

[Krishna]

J'ai encore une petite question et une remarque...
-D'abord merci pour le conseil qui se rapporte aux nombres magiques. Je me demande juste où est-ce qu'il me faut déclarer ce "#define etc... ". J'ai déjà un fichier Variables.h qui devrait d'ailleurs mieux s'appeler constantes.h. Cela fait-il l'affaire?
-Ensuite pour l'utilisation des chaines de caractère, la solution m'a paru simple dans la mesure où les chaines ne renvoient pas à la partie logique de la pièce mais au nom du fichier image qui lui correspond. Il s'agit en fait de la partie graphique de l'échiquier.
Penses-tu dans ces conditions qu'il me faille malgré tout modifier mon code?

[Feriaman]

J'ai déjà un fichier Variables.h qui devrait d'ailleurs mieux s'appeler constantes.h. Cela fait-il l'affaire?

Je suppose que oui, le regroupement des constantes est encore un autre problème de la programmation, mais on ne va pas tous les aborder aujourd'hui.

Sachez juste que si tu mets toutes tes variables au même endroit :
- c'est pratique pour les retrouver
- dès que en change une, tu recompiles presque tout ton programme

-Ensuite pour l'utilisation des chaines de caractère, la solution m'a paru simple ...
Penses-tu dans ces conditions qu'il me faille malgré tout modifier mon code?

J'ai peur d'avoir compris ce que tu dis.

Il faut que tu garde à l'idée que ton modèle de donnée doit être son propre garant d'intégrité. Imagine que demain quelqu'un vient coder avec toi et qu'il fait n'importe quoi (*), et bien tu sera content si tu as un modèle de donnée robuste.

est-il possible de mettre sur ton échiquier une pièce qui ne veut rien dire ("zt" ou "motte_de_beurre") ?
est-il possible de mettre sur ton échiquier une pièce qui se comporte comme un fou, mais qui est affiché comme un cavalier ?
est-il possible que la case [3][4] de ton objet Echiquier contienne une case qui croit qu'elle se trouve en [6][2] ?

Si (comme je le crois) la réponse à l'une (ou à toutes) ces questions est "oui", alors tu n'es pas à l'abri que quelqu'un (*) vienne te péter ton programme parce qu'il ignore (ou qu'il ne se rappelle plus) les règles que tu as fixé.

Je ne sais pas si je suis clair ?



(*) en général, ce type qui fait n'importe quoi, c'est toi-même dans 3 mois quand tu auras oublié toutes les règles implicites que tu t'es fixée.

[Krishna]

Je vais passer mon code au crible de ces conseils judicieux.
Peut-être même chercher un ouvrage sérieux sur la structure des programmes.