Bon, il y a déjà un problème dans le constructeur de Chemin:
Chemin(int ligne, int colonne):m_case(ligne,colonne),m{}
ce devrait être
Chemin(int ligne, int colonne):m_case(ligne,colonne),m_next(NULL){}
car il n'existe aucun membre m dans la classe Chemin 
De même, il faudrait veiller à initialiser m_next à NULL dans le constructeur
Chemin(Case const & c):m_case(c){}
sous la forme de
Chemin(Case const & c):m_case(c),m_next(NULL){}
de manière à être sur que le dernier élément de la liste soit relié à une adresse connue pour son invalidité, au lieu de laisser croire qu'il peut être relié à un élément inexistant
(je soupçonne fortement que ce que le message qui te disait que m n'existe pas portait en réalité sur le premier constructeur et non sur le second 
)
J'aurais aussi voulu avoir la définition de la classe Itineraire, car, c'est elle qui semble te poser le plus de problèmes.
Mais, l'un dans l'autre, voici ce que je peux déjà dire:
Le constructeur
1
2
3
| Itineraire::Itineraire(Chemin* depart,Chemin* arrivee):depart(depart),arrivee(arrivee){
depart->m_next=arrivee;
} |
est, pour le moins, surprenant et n'a (déjà) pas lieu d'être: lorsque tu crée une itinéraire, elle est, par défaut, vide.
Il n'est pas impossible que tu veuille créer un itinéraire en lui passant directement un pointeur sur un chemin d'origine et un autre sur un chemin de destination (respectivement le point de départ et le point d'arrivée), mais, comme la construction d'un objet de type Chemin est prise en charge par l'objet qui sera chargé de sa destruction (autrement dit par un objet de type Itineraire ou de type Robot, uniquement), tu ne peux pas te permettre de te contenter d'assigner l'adresse contenue par les deux pointeurs aux membres respectifs.
En effet, si tu travaille ainsi, tu va te trouver face à une situation problématique dans laquelle deux instances de type Itineraire distinctes partagent leurs pointeurs de type Chemin.
De ce fait, lors de la destruction de la première instance de type Itineraire, la mémoire allouée aux objet de type Chemin sera correctement libérée, mais...
La deuxième instance utilisera toujours les mêmes adresses mémoires, qui ... deviendront invalides.
Dés lors, toute tentative d'accès à ces pointeurs depuis l'instance "survivante" t'enverra "cueillir des pâquerettes", et, lorsque viendra le moment de détruire cette deuxième instance, tu te trouvera face à une tentative de double libération de la mémoire.
En outre, il n'y a strictement rien qui puisse t'inciter à croire qu'il n'y aura jamais que deux éléments à rajouter par cette méthode, et le fait de faire pointer depart->m_next vers arrive occasionnera une belle fuite mémoire et la perte irrémédiable (pour les deux instances) de l'ensemble des éléments se trouvant entre le premier et le dernier.
Tout cela pour dire que, si tu veux donner cette possibilité, tu dois assurer une copie de l'ensemble des éléments se trouvant entre les deux pointeurs transmis en paramètres (ces deux pointeurs étant inclus), un peu à la manière de ce que j'ai fait pour le constructeur par copie de mon intervention précédente
Cela donnerait donc un code proche de
1
2
3
4
5
6
7
8
9
| Itineraire::Itineraire(Chemin * d, Chemin * a):depart(NULL),arrivee(NULL)
{
Chemin * temp = d;
while(d)
{
add(*temp);
++temp;
}
} |
Enfin, pour ce que j'en lis, tu ne profite absolument pas du fait que tu dispose d'un pointeur sur... le dernier élément de la liste lors de l'ajout d'un élément, ce qui est dommage. (cf ma toute première intervention) et tu va, réellement, chercher de la difficulté là où il ne devrait pas y en avoir.
En effet, les lignes de code
1
2
3
4
5
| if((temp->m_case.get_Ligne()!=arrivee->m_case.get_Ligne())||(temp->m_case.get_Col()!=arrivee->m_case.get_Col()))
{
Chemin* auxiliaire=depart;
while(auxiliaire->m_next!=NULL)
auxiliaire=auxiliaire->m_next; |
(qui ne sont décidément pas indentées de manière cohérente) ne servent décidément à rien, vu que tu a la certitude que:
- le premier élément est représenté par depart
- le dernier élément est représenté par arrivee
et cela, même si depart et arrivee pointent en réalité vers... le même objet (ce qui arrive lorsque tu ajoute... le premier élément de la liste)
La fonction add devrait donc prendre la forme de (je ne la réécrit que pour la troisième fois, allez, avec les commentaires pour te permettre de comprendre et en l'adaptant à tes classes existantes 
)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
| void Itineraire::add(Case const & c)
{
/* nous créons dynamiquement un nouveau chemin */
Chemin * temp=new Chemin(c);
/* C'est peut être le premier ajout que l'on fait...
* dans ce cas, notre nouveau chemin devient...
* le premièr de la liste
*/
if(!m_first) // équivalent à if (depart == NULL)
m_first = temp;
/* Si arrivee existe (ce qui revient à dire que ce n'est pas le
* premier ajout) nous relions le dernier chemin à celui nouvellement créé
*/
if(arrivee) // équivalent à if (arrivee != NULL)
arrivee->m_next = temp;
/* En fin de compte, et quoi qu'il advienne, le chemin nouvellement
* créé devient... le dernier de l'itinéraire
*/
arrivee = temp;
} |
(au passage, tu remarquera que je respecte des règles d'indentation strictes: tout ce qui se trouve à un même niveau d'imbriquation commence à la même distance du bord gauche du code )
Et, pour finir, la fonction afficher...
D'abord, je voudrais dire que je ne suis pas vraiment favorable au fait de décider "unilatéralement" que l'affichage se fera d'office sur... la sortie standard.
En effet, tu peux très bien vouloir provoquer l'affichage sur une autre sortie (un fichier texte, par exemple 
)
Or si tu choisi de prendre cette décision unilatérale, lorsque la nécessité de provoquer une sortie vers un fichier texte (pour continuer avec mon exemple), tu devra rajouter une fonction travaillant explicitement sur un fichier mais... ayant exactement le même comportement que ta fonction afficher (hormis le fait qu'elle utilisera un autre flux de sortie).
Dés lors, pourquoi ne pas écrire directement une fonction qui pourra s'adapter à n'importe quel flux de sortie
Cela permettrait même le cas échant, si un jour tu dois passer par un "flux réseau", de garder la même fonction...
Avoue que c'est vraiment de nature à nous simplifier la vie, non
Et ce qui est chouette, c'est que tous les flux de sortie ont une origine commune: la classe ostream.
Ensuite, je voudrais attirer ton attention sur le fait que, idéalement, une fonction d'affichage devrait s'engager... à ne pas modifier l'objet en cours.
Cela rentre dans tout un concept que les anglo-saxons appellent le "const-correctness" (aucune traduction française ne me plait outre mesure) et qui consiste à déclarer constant tout ce qui n'a pas vocation à être modifié.
De cette manière, le compilateur, qui sera toujours plus bûté que toi, refusera toute tentative de modification qui pourrait survenir par erreur alors que tu t'es engagé à ne justement pas apporter de modification.
Pour indiquer au compilateur qu'une fonction membre d'une classe s'engage à ne pas modifier l'objet en cours, il "suffit" de faire suivre la parenthèse fermante par le mot clé const, ce qui nous donnerait, pour la fonction afficher, quelque chose comme
1
2
3
4
5
6
| class Itineraire
{
public:
void afficher() const;
/* tout le reste */
}; |
et une implémentation proche de
1
2
3
4
| void Itineraire::afficher() const
{
/*contenu de la fonction*/
} |
La dernière remarque que je ferai au sujet de cette fonction, c'est que je constate que, là encore, tu n'a pas "purgé" ton code des parties qui n'ont plus lieu d'être...
Je sais que j'ai l'air d'un "vieux con moralisateur", mais, s'il y a une chose dont je tiens à te persuader, c'est que la première qualité d'un code doit être, avant même de faire ce que l'on attend de lui, d'être facilement lisible par la personne qui l'a sous les yeux.
En effet, ce n'est pas un programme qui va devoir apporter des modifications à un code donné (même si certains sont en mesure de le réindenter), mais... un être humain, avec toutes ses qualités et ses faiblesses.
Et cet humain, ce peut être toi, dans trois mois ou un an... ou plus...
Or, le code que tu écris aujourd'hui te semble "parfaitement clair" là, à l'instant, parce que tu l'a bien en tête, mais, dans trois mois, alors que tu auras écrit des centaines ou des milliers d'autres lignes de code (ou passé des heures à te casser la tête sur d'autres cours, ou ...), si ton code est truffé de partie "désactivées" (parce que mises en commentaire), s'il ne respecte pas une politique stricte de mise en forme, s'il ne contient pas des commentaires clairs sur ce que tu veux faire, si les noms de variables et de fonctions ne sont pas explicites, tu va perdre beaucoup plus de temps à essayer de "te rappeler" de ce que tu voulais faire que ce que tu *crois* gagner aujourd'hui comme temps à écrire "à la va vite".
Et encore, je ne parle ici que d'un code que tu écris toi-même, mais, imagine si le code est écrit par quelqu'un d'autre...
Enfin bref, cela n'apporte pas encore de solution à ton problème, mais je tenais malgré tout à attirer ton attention sur ce point
La première chose qui me choque à la lecture du code de ta fonction afficher (ou plutôt la première grosse erreur, dirons nous ), c'est que... tu crées un nouvel objet de type Chemin, alors que tu n'en a absolument pas besoin.
Et, comme tu te base sur ce nouvel objet pour toute la suite, fatalement, tout le reste de la logique qui suit devient tout à fait foireux. (sans compter le fait que ce nouvel objet n'est jamais détruit, et que l'on assiste donc à une fantastique fuite mémoire )
En effet, la logique consiste, tout simplement, à - prendre le premier élément de la liste,
- le faire afficher
- puis à passer au suivant,
- recommencer en (2) jusqu'à ce qu'il n'y ait plus d'élément à afficher
En prenant toutes ces remarques en compte, la fonction affiche pourrait être définie comme suit:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
| void Itineraire::afficher(ostream &ofs) const
{
/* d'abord, nous déclarons un pointeur de travail que nous
* initialisons avec l'adresse contenue par... le premier élément
* de la liste
*/
Chemin * travail = depart;
/* tant qu'il y a un élément à afficher, nous entrons dans la boucle
*/
while(travail) // équivalent à while (travail !=NULL)
{
/* nous envoyons dans le flux sortant les informations concernant
* la case contenue par le chemin en cours
*/
ofs<< "("<<travail->m_case.get_Col()
<<","<<travail->m_case.get_Linge()
<<")"<<std::endl;
/* et nous passons à l'élément suivant */
++travail;
}
} |
Normalement, et pour autant que le code que tu nous a présenté soit effectivement le code sur lequel tu te base (n'oublie pas qu'il faut relancer la compilation chaque fois que tu apporte une modification au code ), si tu apporte les quelques modifications que je viens de te présenter, il n'y a aucune raison pour que ton code ne compile pas, ni pour qu'il ne réagisse pas exactement comme tu l'attend.
ajout d'un élément à une liste (structure)
Répondre avec citation
A méditer: La solution la plus simple est toujours la moins compliquée
Envoyé par dingua
Partager