Discussion :

[yo_haha]

Bonsoir,
J'aimerais que vous m'aidez à corriger le code suivant:
UnePiece.h

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
#ifndef _UNEPIECE_H
#define _UNEPIECE_H
#include<string.h>
 
class UnePiece
{
protected:
	char *sonNom;
public:
	UnePiece(char *tel_ptr)
	{
		sonNom = NULL;
		sonNom = new char[strlen(tel_ptr)+1];
		strcpy(sonNom,tel_ptr);
	};																																	}*/
	~UnePiece()
	{
		delete[] sonNom;	
   };
#endif

UnePieceSimple.h

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
#ifndef _UNEPIECESIMPLE_H
#define _UNEPIECESIMPLE_H
#include "UnePiece.h"
#include <string.h>
 
class UnePieceSimple: public UnePiece
{
private:
   int son_poids;
 
public:
	UnePieceSimple(char *tel_ptr ,int tel_pds): UnePiece(tel_ptr), son_poids(tel_pds){}
 
   int donnePoids();
 
   void affiche(int);		
};
 
 
class UnePieceComposite :public UnePiece
{
private:
    UnePiece **sesComposants; // fin du tableau marquée par NULL
 
public:
	 UnePieceComposite(char *tel_ptrn , UnePiece **tel_ptrp) :
    UnePiece(tel_ptrn)
    {
		int i,j;
		for (i=0; tel_ptrp[i]; i++);
	   sesComposants = new UnePiece*[i+1];
	   for (j=0;j<=i;j++)
	     sesComposants[j]=tel_ptrp[j];
    }
 
	 int donnePoids();
 
	 void affiche(int);
 
	 ~UnePieceComposite ()
	 {
		delete []sesComposants;
	 }
};
#endif

UnePieceSimple.cpp

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
#include "UnePiece.h"
#include "UnePieceSimple.h"
#include <iostream>
using namespace std;
#include <string.h>
 
int UnePieceSimple::donnePoids(){return son_poids;}
 
void UnePieceSimple::affiche(int n)
{
	int i;
	for (i=0; i<n; i++) 
     cout << "\t";
	cout << son_nom << endl;
}
 
 
int UnePieceComposite::donnePoids()
{
	int som=0,i=0;
	while(sesComposants[i] != NULL)
   {
		som+=sesComposants[i]->donnePoids();
		i++;
	}
	return som;
}
 
 
void UnePieceComposite::affiche(int n)
{
   int i;
	for(i=0;i<n;i++) 
     cout<<"\t";
	cout << son_nom <<":"<< endl;
	i=0;
   while(sesComposants[i] != 0)
   {
   	sesComposants[i]->affiche(n+1);
		i++;
	}
}

main.cpp

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
#include <cstdlib>
#include <iostream>
#include "UnePieceSimple.h"
 
using namespace std;
 
int main(int argc, char *argv[])
{
 
    UnePieceSimple la_1("ecrou", 14);
    UnePieceSimple la_2("vis", 12);
    UnePiece **tab_1 = {&la_1, &la_2, NULL};
    UnePieceComposite la_3("appareil1", tab_1);
    UnePiece **tab_2 = {&la_1, &la_2, &la_3, NULL};
    UnePieceComposite la_4("appareil2", tab_2);
 
    la_4.donnePoids();
    la_4.affiche(1);
 
 
    system("PAUSE");
    return EXIT_SUCCESS;
}

Est-ce que les fonctions donnePoids() et affiche(int) doivent être virtuelles? Moi je pense que oui parce que sesComposants est un tableau de pointeurs de type UnePiece sur des objets de types UnePieceSimple et UnePieceComposite.
Y a-t-il une erreur avec les destructeurs? Je voulais le vérifier mais plusieurs erreurs dont la cause est ignorée sont apparues:

Code :

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

1
2
3
4
5
6
7
8
9
3 C:\Users\user\Desktop\kk\UnePieceSimple.h:3,               from main.cpp In file included from UnePieceSimple.h:3,               from main.cpp 
3 C:\Users\user\Desktop\kk\main.cpp                  from main.cpp
15 C:\Users\user\Desktop\kk\UnePiece.h expected unqualified-id before '/' token 
15 C:\Users\user\Desktop\kk\UnePiece.h expected init-declarator before '/' token 
15 C:\Users\user\Desktop\kk\UnePiece.h expected `,' or `;' before '/' token 
 C:\Users\user\Desktop\kk\main.cpp In function `int main(int, char**)': 
12 C:\Users\user\Desktop\kk\main.cpp initializer for scalar variable requires one element 
14 C:\Users\user\Desktop\kk\main.cpp initializer for scalar variable requires one element 
 C:\Users\user\Desktop\kk\Makefile.win [Build Error]  [main.o] Error 1

[Antoine_935]

Salut

Voici les erreurs que je vois, mais il est possible que certaines m'aient échappé

- Il manque une accolade fermante après la définition du destructeur pour UnePiece.
- Le compilateur indique qu'il a rencontré un '/' inattendu. Aurais-tu enlevé des commentaires de ce fichier ? Quoi qu'il en soit, vérifie que tu utilises bien un compilateur de C++, cette erreur me fait penser à un compilo C en mode ANSI.
- Dans le constructeur de UnePieceComposite, tu as un for directement suivi d'un point virgule. Le compilo considère ça comme un for qui ne contient pas d'instruction.
J'avais mal lu, mais tu aurais du donner un autre nom à la variable i, comme longueur ou len par exemple
- Dans ton main, les deux lignes où tu crées des tableaux, remplace la définition de la variable par UnePiece* tab[ ], il est possible que le compilateur se formalise pour ça.

Et voici quelques notes supplémentaires, fais en ce que tu veux.
- c'est une mauvaise idée de mettre l'attribut "sonNom" en protected. Tu préféreras private.
- il est inutile d'assigner NULL à sonNom, alors que juste après tu redéfinis sa valeur.
- choisis des noms de variables plus explicites, sans quoi la relecture sera laborieuse voire même horrible. De plus, les variables nommées i et, dans une moindre mesure, j devraient être réservées à l'itération pure.
- N'hésite pas à mettre des espaces et passages à la ligne pour séparer les groupes, ça aide grandement à la lecture. Idem pour l'indentation rigoureuse.

Et pour répondre à tes questions sur les méthodes virtuelles, jamais un attribut de la classe ne justifierait des virtual.
Les virtuals s'employent en cas d'héritage, pour pouvoir redéfinir ladite méthode dans la classe enfant. Or ici, aucune class n'hérite de UnePieceSimple, où sont définies ces deux méthodes.

Ce que tu veux faire je crois, c'est définir des méthodes qui seraient accessibles dans tous les enfants de UnePiece. Dans ce cas, c'est dans UnePiece que tu dois déclarer ces méthodes comme étant virtuelles.

[Davidbrcz]

Je n'ai pas le temps de tester le code pour trouver les erreurs syntaxiques (je fais confiance à Antoine_935) mais je te conseille de regarder du coté de la STL pour remplacer les char* par des std::string et tes tableaux de char* par des std::vector<std::string> (pour connaitre l'utilisation, cf la FAQ C++).

Enfin, note que dans l'état actuel, il faut à ta classe un constructeur de recopie et un opérateur d'affection (ou alors les interdires) sans quoi, tu vas recevoir un segfault assez rapidement. Pour savoir comment faire une recherche sur le forum, google et la FAQ.

[yo_haha]

- Le compilateur indique qu'il a rencontré un '/' inattendu. Aurais-tu enlevé des commentaires de ce fichier ?

Effectivement, il y avait un "*/" quelque part.

- Dans le constructeur de UnePieceComposite, tu as un for directement suivi d'un point virgule. Le compilo considère ça comme un for qui ne contient pas d'instruction.

Pourquoi un for sans instructions est mal vu? Je l'ai utilisé pour calculé le nombre d'éléments du tableau à l'aide de i.

J'avais mal lu, mais tu aurais du donner un autre nom à la variable i, comme longueur ou len par exemple

Le i est utilisé comme compteur dans for et au même temps il renseigne sur le nombre d'éléments du tableau.

- c'est une mauvaise idée de mettre l'attribut "sonNom" en protected. Tu préféreras private.

sonNom est un champ de la classe mère qui sert aux classes filles. "Il doit être protected" c'est la règle, non?

- il est inutile d'assigner NULL à sonNom, alors que juste après tu redéfinis sa valeur.

Tu as raison

Ce que tu veux faire je crois, c'est définir des méthodes qui seraient accessibles dans tous les enfants de UnePiece. Dans ce cas, c'est dans UnePiece que tu dois déclarer ces méthodes comme étant virtuelles.

C'est exactement ce que je voulais dire. Je vais écrire l'affirmation suivante et je veux savoir si elle est correcte: "Quand on lance l'instruction suivante

Code :

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

la_4.affiche(1);

si la méthode affiche(int) est virtuelle avant de l'exécuter, le compilateur vérifie le type de l'objet sur lequel il va appliquer la méthode (puisque la_4 est composée de plusieurs pieces simples et composites) pour voir s'il va utiliser affiche(int) de UnePieceSimple ou bien de UnePieceComposite. Si affiche(int) n'est pas virtuelle et elle est définie dans UnePiece et surdéfinie dans UnePieceSimple et dans UnePieceComposite alors c'est la méthode de UnePiece qui sera exécutée car sesComposant est de type UnePiece**. Dans notre cas, puisque affiche(int) n'est définie que dans UnePieceSimple et dans UnePieceComposite va attendre la vérification du type de l'objet visé pour appliquer la méthode selon le type de l'objet."

J'ai encore une dernière erreur: multiple types in one declaration. L'erreur est signalée pour la ligne qui contient l'accolade fermante de la déclaration de la classe UnePieceSimple.

Finalement merci de tes conseils.

[Antoine_935]

Pourquoi un for sans instructions est mal vu? Je l'ai utilisé pour calculé le nombre d'éléments du tableau à l'aide de i.

C'est pas forcément "mal vu", c'est juste qu'après un for on attend au moins une instruction, par habitude. Au moins mets un commentaire au dessus pour avertir et dire ce qu'il fait.

Le i est utilisé comme compteur dans for et au même temps il renseigne sur le nombre d'éléments du tableau.

Oui, mais il représente plus la longueur du tableau que l'élément qu'on est en train de traiter. C'est un avis personnel évidemment.

sonNom est un champ de la classe mère qui sert aux classes filles. "Il doit être protected" c'est la règle, non?

Hehe, ça me rappelle un prof en début d'année :p Il avait crié parce qu'un élève avait suggéré de mettre un attribut en protected, au cours d'uml. On l'a pris pour un fou sur le moment même, mais mnt on se rappelle tous qu'un attribut protected c'est NAN !
Bref non, pas d'attributs en protected, pour une raison simple: si un jour tu dois changer l'implémentation de la classe UnePiece et sa représentation du nom (pas bcp de chances dans ce cas, d'accord), tu devras retravailler les classes enfant.
La solution est de mettre des méthodes publiques getNom() et éventuellement setNom() (getter / setter classique). Tu te serviras ensuite de ce getter dans la classe enfant quand tu as besoin du nom.

C'est exactement ce que je voulais dire. Je vais écrire l'affirmation suivante et je veux savoir si elle est correcte: "Quand on lance l'instruction suivante

Code :

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

la_4.affiche(1);

si la méthode affiche(int) est virtuelle avant de l'exécuter, le compilateur vérifie le type de l'objet sur lequel il va appliquer la méthode (puisque la_4 est composée de plusieurs pieces simples et composites) pour voir s'il va utiliser affiche(int) de UnePieceSimple ou bien de UnePieceComposite. Si affiche(int) n'est pas virtuelle et elle est définie dans UnePiece et surdéfinie dans UnePieceSimple et dans UnePieceComposite alors c'est la méthode de UnePiece qui sera exécutée car sesComposant est de type UnePiece**. Dans notre cas, puisque affiche(int) n'est définie que dans UnePieceSimple et dans UnePieceComposite va attendre la vérification du type de l'objet visé pour appliquer la méthode selon le type de l'objet."

Dans notre cas, le compilateur ne va tout simplement pas l'accepter, puisque sesComposants est un tableau de UnePiece, et que dans UnePiece il n'y a pas de méthode affiche(int).

Donc oui, tu dois déclarer une virtuelle, éventuellement pure, dans UnePiece.

[Goten]

Les getters/setter sont pas beaucoup mieux d'un point de vue conception te diront certains. Pour ce qui est du protected je partage pas l'avis de ton prof ...

[Antoine_935]

Les getters/setter sont pas beaucoup mieux d'un point de vue conception te diront certains. Pour ce qui est du protected je partage pas l'avis de ton prof ...

Si tu le dis, mais... explique ton avis.

[Davidbrcz]

Si tu le dis, mais... explique ton avis.

On encapsule des comportements, pas des données.
Cf ici

[Antoine_935]

Sans oublier bien sur les deux phrases qui précèdent:

En fait, encapsuler seulement l'accès aux propriétés d'un objet est aussi inneficace que de mettre ces propriétés en accès public. Ce que l'on a besoin d'encapsuler, c'est la manière dont on se sert de ces propriétés. D'où la règle suivante:

On encapsule un comportement, pas des propriétés.

Ce que le rédacteur voulait dire, c'est qu'il faut encapsuler à la fois les données et le comportement.

Le but de ça c'est éviter les changements futurs dans le code. Si on peut modifier la classe parent sans toucher aux enfants, tant mieux. Voila pourquoi les attributs devraient rester privés.

C'est mon avis bien sur, faites en ce que vous voulez. Quoi qu'il en soit, il est toujours possible de se passer d'attributs protected.

Les méthodes protected sont un bienfait par contre, parfois.
Imaginez un pattern observer simplifié:
Une classe sujet abstraite TextModel, qui a pour méthodes publiques addListener et removeListener, et stocke ces listeners dans une liste.

De cette classe héritent divers autres modèles concrets. Lorsque ces enfants doivent envoyer un événement aux listeners, ils ont deux choix:
- soit accéder à la liste (protected) de la classe parent, et envoyer un message à chaque listener
- soit appeler la méthode protected fireEvent de la classe parent (liste privée)

Dans le premier cas, si la méthode de stockage des listeners vient à changer dans la classe parent, on peut modifier tous les enfants.
De plus, ça laisse une ouverture aux erreurs de programmation dans les classes enfant même, comme une éventuelle modification de la liste, erreur de sync ou autre.

Dans le deuxième cas, il suffit d'appeler une méthode. Ca tient sur une ligne, on sait que ça fonctionne et on n'a pas besoin de savoir ce qui se passe dans une autre classe. Ce n'était pas notre boulot, après tout.


Voila pour l'idée que je me fais des attributs protected. Pour ma part, je ne m'en sers jamais, me porte très bien et mon code aussi


PS: pour ceux qui répondraient qu'un appel de méthode coute du temps à l'exécution, pensez à inline.

[Davidbrcz]

J'ai posté rapidement, j'étais pressé, je reviens à la charge.

Sans oublier bien sur les deux phrases qui précèdent:
Ce que le rédacteur voulait dire, c'est qu'il faut encapsuler à la fois les données et le comportement.

Non, on encapsule vraiments que des comportements, les données ne sont qu des conséquences nécéssaire des ces comportements.

Quand on écrit une classe, on expose publiquement des méthodes pour modifier le comportement de l'objet. Exemple avec une voiture:
Une voiture peut accélerer,décélérer et tourner. Elle offre les méthodes permetant de modifier sa trajectoire (en module et direction). Pour implémenter ces possibilités, on utilise des données (vitesse, accélération, position, ...) mais qui, je le répète, ne sont que des conséquences de actions offertes par l'interface.

Le but de ça c'est éviter les changements futurs dans le code. Si on peut modifier la classe parent sans toucher aux enfants, tant mieux. Voila pourquoi les attributs devraient rester privés.

On devrait toujours pouvoir modifier le parent sans toucher aux enfants.
L'inconvénient d'utiliser des attributs privés, c'est que ca oblige à avoir des getter/setter (protected) pour accéder directement aux données (et des fois, y'en a besoin)

Les méthodes protected sont un bienfait par contre, parfois.
Imaginez un pattern observer simplifié:
Une classe sujet abstraite TextModel, qui a pour méthodes publiques addListener et removeListener, et stocke ces listeners dans une liste.

De cette classe héritent divers autres modèles concrets. Lorsq&ue ces enfants doivent envoyer un événement aux listeners, ils ont deux choix:
- soit accéder à la liste (protected) de la classe parent, et envoyer un message à chaque listener
- soit appeler la méthode protected fireEvent de la classe parent (liste privée)

Dans le premier cas, si la méthode de stockage des listeners vient à changer dans la classe parent, on peut modifier tous les enfants.
De plus, ça laisse une ouverture aux erreurs de programmation dans les classes enfant même, comme une éventuelle modification de la liste, erreur de sync ou autre.

Dans le deuxième cas, il suffit d'appeler une méthode. Ca tient sur une ligne, on sait que ça fonctionne et on n'a pas besoin de savoir ce qui se passe dans une autre classe. Ce n'était pas notre boulot, après tout.

C'est une bonne idée. Un autre exemple de fonction membre protected, c'est le pattern NVI (non virtual interface)

PS: pour ceux qui répondraient qu'un appel de méthode coute du temps à l'exécution, pensez à inline.

Bof. Mal utilisé, inline ralentie plus qu'autre chose. Mais de toute façon, inline n'est qu'une directive au compilateur, directive qu'il peut suivre ou non.

[Antoine_935]

Non, on encapsule vraiments que des comportements, les données ne sont qu des conséquences nécéssaire des ces comportements.

Oui, mais pour reprendre ton exemple, si d'un coup tu inverses la vitesse de la voiture, qu'elle passe de 120Km/h à la marche arrière, c'est surtout un problème de cohérence qui va se poser. D'où l'intérêt d'interdire l'accès direct à ces variables. D'autre part, et comme tu le suggère, ce sont bien des appels de méthodes qui doivent modifier ces valeurs.

On devrait toujours pouvoir modifier le parent sans toucher aux enfants.
L'inconvénient d'utiliser des attributs privés, c'est que ca oblige à avoir des getter/setter (protected) pour accéder directement aux données (et des fois, y'en a besoin)

Trois lignes par getter ou setter, c'est pas cher payé

C'est une bonne idée. Un autre exemple de fonction membre protected, c'est le pattern NVI (non virtual interface)

Beau pattern tiens, je ne le connaissais pas. Merci de m'avoir fait découvrir ça
Cela dit, selon l'article que je viens de lire, et le code écrit, les méthodes virtuelles ne doivent pas nécessairement être protected: private fonctionne tout aussi bien
http://www.gotw.ca/publications/mill18.htm

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
#include <iostream>
using namespace std;
 
class Interface {
private:
        virtual void act() {
                cout << "Interface" << endl;
        }
 
public:
        void process() {
                act();
        }
};
 
class Implement: public Interface {
        void act() {
                cout << "Implement" << endl;
        }
};
 
int main() {
        Interface* f = new Implement();
 
        f->process();
 
        return 0;
}

Bof. Mal utilisé, inline ralentie plus qu'autre chose. Mais de toute façon, inline n'est qu'une directive au compilateur, directive qu'il peut suivre ou non.

C'est vrai, mais dans le cas d'un simple return ou assign, ça ne pose pas de problème je crois

[Davidbrcz]

Oui, mais pour reprendre ton exemple, si d'un coup tu inverses la vitesse de la voiture, qu'elle passe de 120Km/h à la marche arrière, c'est surtout un problème de cohérence qui va se poser.

Un bon mur ca fait de miracles

D'où l'intérêt d'interdire l'accès direct à ces variables. D'autre part, et comme tu le suggère, ce sont bien des appels de méthodes qui doivent modifier ces valeurs.

Voui, ce sont bien les fonctions de l'interface qui doivent modifier la valeur, pas des getters/setters artificiels.

Trois lignes par getter ou setter, c'est pas cher payé

Mouais, ca reste quand même 3 de trop.

Beau pattern tiens, je ne le connaissais pas. Merci de m'avoir fait découvrir ça

Il devrait bientôt être intégré dans la FAQ.

Cela dit, selon l'article que je viens de lire, et le code écrit, les méthodes virtuelles ne doivent pas nécessairement être protected: private fonctionne tout aussi bien
http://www.gotw.ca/publications/mill18.htm

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
#include <iostream>
using namespace std;
 
class Interface {
private:
        virtual void act() {
                cout << "Interface" << endl;
        }
 
public:
        void process() {
                act();
        }
};
 
class Implement: public Interface {
        void act() {
                cout << "Implement" << endl;
        }
};
 
int main() {
        Interface* f = new Implement();
 
        f->process();
 
        return 0;
}

Private ou protected ce n'est quand même pas la même chose.

Ici, tu appelles la méthode Interface::process depuis un objet du type Implement. process réalise un appel à act qui est une fonction virtuelle, le choix de la bonne fonction se fait donc selon le type réel de l'objet => la fonction appelé sera Implement::act. Ca marche, ok.

Mais tente d'appeler act depuis une fonction membre foo de Implement sans l'avoir redéfinie => Epic fail !

C'est vrai, mais dans le cas d'un simple return ou assign, ça ne pose pas de problème je crois

Nan, c'est vrai, car en plus, ya les optimisations du compilo après.

[Antoine_935]

Un bon mur ca fait de miracles

Mais tente d'appeler act depuis une fonction membre foo de Implement sans l'avoir redéfinie => Epic fail !

Oui, c'est sur, private quoi