C'est mieux... mais tu aurais pu te contenter d'un copier /coller du contenu des fichiers "importants" (pile.h, elt.h, pile.cpp et...main.cpp
(il est manquant ) ) directement sur le forum, en prenant soin d'encadrer le code avec les balises [ CODE ] et [ /CODE ] (le bouton # qui se trouve au dessus du formulaire de question /réponse )
Et, un rapide survol de ton code me fait dire qu'il y a, vraiment, place pour le progres 
Pour la facilité de tous, je vais faire ce copier coller, du moins pour les fichiers dont on dispose, avant de faire mes différentes remarques:
elt.h original
1
2
3
4
5
6
7
8
9
10
11
12
13
14
| //Definition (implementation) d'un élément qui formera la pile
#ifndef _ELT_H_
#define _ELT_H_
typedef struct elet
{
int valeur;
elet *suivant;
}
elt;
#endif |
pile.h original
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
| //Fichier .h de la classe pile
#ifndef _PILE_H_
#define _PILE_H_
#include "elt.h"
class pile
{
elt *un_elt;
//Methodes
public:
//Methode pour depiler
elt* empiler(elt *lapile,int a);
//Methode pour lire les valeurs
void lire();
//Methode pour depiler
elt* depiler(elt *lapile);
//fonction pour Afficher
void Afficher();
};
#endif |
pile.cpp original
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
| #include <iostream>
#include "elt.h"
#include "pile.h"
using namespace std;
//Methode pour Empiler
elt* pile :: empiler (elt *lapile,int val)
{ elt *p,*elm;
if(lapile==NULL)
{lapile=(elt*)malloc(sizeof(elt));
(*lapile).valeur=val;
(*lapile).suivant=NULL;
}
else
{
elm=(elt*)malloc(sizeof(elt));
(*elm).valeur=val;
(*elm).suivant=lapile;
lapile=elm;
}
return lapile ;
}
//Methode pour lire les valeurs
void pile::lire()
{elt *pile;
int i,n,valeur;
cout<<"\nDonner la taille de la pile\n";
cin>>n;
pile=NULL;
for(i=1;i<=n;i++)
{cout<<"entrer la valeur \t"<<i;
cin>>valeur;
pile=pile::empiler(pile,valeur);
}
}
////Methode pour Depiler
//elt* pile ::Depiler (elt *lapile)
// { elt *p;
// p=lapile;
// lapile=(*lapile).suivant;
// free(p);
// return lapile;
//}
//Methode pour Afficher
void pile::Afficher()
{
pile* p=this;
while(p!=NULL)
{
cout<<"\nLa valeur est \n"<< (*un_elt).valeur;
un_elt=(*un_elt).suivant;
}
}
// void pile :: Afficher( elt *lapile)
// { elt *p;
// p=lapile;
// while(p!=NULL)
// { cout<<"\nLa valeur est \n"<< (*p).valeur;
// p=(*p).suivant;
// }
// } |
Avant d'aller plus loin, si tu dois pisser, sortir le chien, fais le, prend ton paquet de clopes (si tu fumes) à coté de toi, sers toi un café (ou ce que tu veux d'autre), car je prévois de faire encore un post qui entrera dans les annales et qui me vaudra (encore une fois) d'être chambré par mes pairs...
Pour commencer, je vais faire les remarques d'ordre général que m'inspirent ton code...
Il faut, en premier lieu, savoir que, contrairement au C, toute définition de type (qu'il s'agisse d'un structure, d'une classe, d'une union ou d'une énumération) rend automatiquement le type disponible à l'utilisation.
Il ne sert donc à rien de rajouter un typedef lorsque tu définit une strucutre.
Un simple
1
2
3
4
5
6
7
|
struct MaStructure
{
/*blabla */
/* tu peux même utiliser directement MaStructure en interne */
MaStructure * truc;
}; |
suffit.
De plus, il faut savoir qu'il n'y a que très peu de différence entre les classes et les structures en C++...
Elles tiennent principalement à l'accessibilité qui est appliquée par défaut à leur contenu (cf l'entrée de la FAQ qui en parle)
Pour travailler "entièrement" en orienté objet, nous pourrions donc envisager de créer non pas une structure de type el(e)t mais... carrément une classe dont je parlerai en temps utiles.
Ensuite, je voudrais attirer ton attention sur deux points que j'ai oublié de citer dans les conseils pour garder un code lisible et que ton code met en évidence:
Tous deux concernent les commentaires...
Les commentaires sont très utiles, pour ne pas dire indispensables dans certains cas.
Mais il vaut mieux se passer de commentaire que d'en avoir de mauvais.
Un "bon" commentaire sera un commentaire qui apporte une information qu'une lecture rapide du code ne met peut être pas en évidence.
Il ne faut pas vraiment sortir de Math Sup pour comprendre qu'une fonction nommée explicitement empiler va servir à ... empiler des objets... On se doute que le but de la fonction n'est pas de partir à la chasse au papillons 
De même un commentaire proche de (je te rassure, il n'y en a pas dans ton code 
)
1
2
3
|
int i;
i++; // incrémente i |
n'a en définitive aucun intérêt: il n'appporte aucune indication que le code ne donne pas.
Par contre, si l'explication vise à expliquer pourquoi on décide d'incrémenter i, le commentaire peut prendre tout son sens (car l'incrémentation peut sembler aller "à contre courent" de la logique de la fonction)
Le deuxième conseil que je veux rajouter découle directement du premier:
Ne laisse pas du code "périmé" et commenté dans ton code.
Il peut arriver que l'on souhaite "désactiver" une portion de code le temps d'effectuer un test, et il est donc "envisageable" de commenter cette portion le temps de faire le test en question.
Mais ce code ne doit pas rester si son inutilité est avérée et que son retrait ne provoque aucun problème: Soit la portion désactivée est nécessaire, et il faut la "dé-commenter", soit elle n'est pas nécessaire, et il faut la supprimer:
La présence de ce code désactivé complique en effet fortement la relecture, que ce soit en obligeant le lecteur à jouer avec les ascenseurs pour accéder à la suite ou, pire encore (on ne lit pas forcément un code avec un outil disposant de la coloration syntaxique et certains ont même ce genre d'outils en horreur) ne pas être interprété comme tel par le lecteur et l'induire en erreur sur ce qui est *réellement* fait.
Je constate, en outre, que tu utilise la fonction C malloc pour tes allocation dynamiques...
Ce n'est pas à proprement une faute dans le sens où cette fonction existe aussi en C++, mais c'est néanmoins une erreur dans le sens où, à l'instar de nombreuses autres choses issues du C, C++ propose une alternative beaucoup plus facile et sécurisante à utiliser: new, dans le cas présent.
En effet, alors que malloc se contente de renvoyer un pointeur vers la mémoire allouée dynamiquement (ou NULL si l'allocation a échoué), new permet d'appeler directement le constructeur de la classe, et donc d'obtenir directement un objet correctement initialisé selon les règles que nous avons décidé (nous approchons du concept RAII (Ressource Acquizition Is Initialisation, L'acquisition de ressources vaut initialisation si tu préfère en français) ), mais, en plus, mais, en plus, il ne renverra le pointeur que si l'allocation a bel et bien eu lieu (il lancera une exception de type bad_alloc si elle a échoué).
Et, cerise sur le gâteau, la syntaxe est autrement plus simple:
En effet, là où tu dois écrire un code proche de
Type * ptr = (Type*) malloc(sizeof(Type));
avec malloc, tu peux te contenter d'un simple
voire, si tu dois passer des arguments pour contruire ton objet d'un
Type *ptr= new Type(/* les arguments nécessaires*/)
Evidemment, si on "renonce" à malloc au profit de new, il faut renoncer à free au profit de delete (il ne faut en aucun cas appeler free sur un pointeur alloué par new ni appeler delete sur un pointeur alloué par malloc).
Et, bien sur, delete présente lui aussi un avantage certain par rapport à free: il appelle automatiquement le destructeur de l'objet avant de libérer la mémoire allouée, ce qui provoque la libération correcte et en profondeur des ressources allouées à l'objet
Pour en terminer avec les généralités, je voudrais, après avoir insisté une fois de plus sur l'importance de respecter une politique d'indentation stricte et cohérente, te parler de la manière dont tu accède aux éléments de tes pointeurs.
En effet, tu y accède systématiquement sous la forme de
qui, bien que n'étant pas tout à fait fausse me semble être une habitude par laquelle tu te fait du mal inutilement...
En effet, il existe une syntaxe particulière pour accéder au contenu d'un pointeur: la fleche " -> "...
Dés lors, pourquoi se faire du mal à vouloir prendre "ce qui est pointé par" ton pointeur avant d'essayer d'accéder à son contenu
D'autant plus que, pour l'instant, tu as de la chance : tu tente tout de suite d'accéder... au contenu du pointeur, mais quid si tu essayais d'accéder au contenu d'un pointeur qui est lui-même contenu dans un autre objet vers lequel tu dispose d'un pointeur
Ne crois tu pas qu'il est presque plus rapide d'utiliser la fleche sous la forme de
lePointeur->ptr->truc/*...*/;
plutôt que ta méthode, qui se traduirait par un
(*lePointeur).(*ptr).truc/*...*/
Et ce serait encore pire si tu essayais d'accéder au contenu d'un pointeur qui est lui même contenu dans un pointeur renvoyé par la fonction membre d'un objet dont tu ne dispose que sous la forme d'un pointeur 
De plus, bien que n'étant pas erronée, ta méthode présente un inconvénient majeur : elle va à l'encontre des pratiques habituelles de la communauté en générale.
De ce fait, tu t'éloigne de la première qualité d'un code qui est d'être facilement lisible par "le plus grand nombre"...
Bien sur, tout programmeur digne de ce nom comprendra ton code, mais disons que, tu risque de voir des points d'interrogation voler autour de sa tête le temps qu'il "percute" sur ton habitude peu orthodoxe...
Ceci étant dit, je voudrais pointer du doigt la logique assez particulière que tu mets en oeuvre dans ta fonction empiler (hé oui, tu t'es un peu avancé en criant bien haut que la fonction empiler fonctionnait correctement)...
A vrai dire, j'ai un peu de mal à comprendre pourquoi tu passe un pointeur sur ce qui, de toutes évidences, est destiné à devenir... le dernier élément emplié, alors que ta pile dispose d'un tel pointeur sous la forme de ... un_elt, et, surtout, pourquoi tu travaille essentiellement avec le pointeur passé en argument et non avec... un_elt... qui n'est jamais utilisé dans la fonction empiler (et dont je doute qu'il le soit dans la fonction depiler, mais, comme tu as eu l'extrême amabilité de nous en cacher le code...
)
Si c'est pour garder artificiellement le pointeur représentant le haut de la pile en dehors de la pile, à quoi peut donc bien servir ce membre un_elt , et, par voie de conséquence, pourquoi créer une classe pile alors que tout ce qui semble t'intéresser pourrait prendre la forme de fonctions libres
Finalement, j'ai changé d'avis...
Au départ, j'avais en tête de te présenter point par point une manière correcte et efficace de mettre ta pile au point, mais, je me rend compte que mon intervention prend déjà l'allure d'un véritable roman (et que je suis déjà sur de me faire chambrer)...
De plus, je me dis qu'il te faudra un certain temps pour assimiler les différentes remarques que j'ai faite et pour arriver à les appliquer de manière cohérente dans ton code.
Enfin, je me dis que ce serait peut être un très mauvais service à te rendre que de *trop* te mâcher la tâche (sans oublier que ce serait récompenser un peu facilement ton manque de collaboration).
Je vais donc te laisser réfléchir un peu à tout cela, et j'attends de tes nouvelles pour voir comment tu aura pris ces remarques en compte, voire, te donner l'occasion de poser des questions supplémentaires sur celles-ci, avant de risquer de te larguer complètement avec la suite des explications...
afficher une pile en POO
Répondre avec citation
A méditer: La solution la plus simple est toujours la moins compliquée
Envoyé par lcfseth
Partager