Return à l'intérieur d'une boucle for -> warning

Bonjour,

j'ai une classe Element de la sorte :

Code:

---

1 2 3 4 5 6

class Element { public: std::string m_id; int m_value };

---

j'ai un vecteur d'éléments de type Element :

Code:

---

std::vector< Element * > * theVectorOfElements;

---

j'ai une fonction de recherche d'un élément Element dans le vecteur en fonction de son identifiant :

Code:

---

Element * searchElement( std::string anId , std::vector< Element * > * aVectorOfElements );

---

sachant que :
- tous les attributs m_id des différents éléments du vecteur sont distincts
- lorsque je fais une recherche pour anId, je suis sûr qu'il existe bien un élément du vecteur qui à comme attribut m_id cet valeur anId (ce qui suppose en plus que ce vecteur n'est pas vide.

j'ai donc implémenté cette fonction de la manière suivante :

Code:

---

1 2 3 4 5 6 7 8 9 10 11

Element * searchElement( std::string anId , std::vector< Element * > * aVectorOfElements ) { std::vector< Element * >::iterator anIter = aVectorOfElements->begin(); for( ; anIter != aVectorOfElements->end() ; ++anIter ) { if( (*anIter)->m_id == anId ) { return( (*anIter) ); } } }

---

Or j'ai un warning à la compilation :
"warning: control reaches end of non-void function"
dû au fait que le "return" est dans la boucle.
Comme ce vecteur est très grand et selon mes hypothèses, je préfère ne pas avoir à le parcourir en entier si pas besoin. Comment puis-je faire pour éviter ce warning ?

Peut-être en faisant une boucle while de la sorte (avec une variable temporaire booléenne) :

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16

Element * searchElement( std::string anId , std::vector< Element * > * aVectorOfElements ) { Element * theReturnedElement = NULL; bool isFind = false; std::vector< Element * >::iterator anIter = aVectorOfElements->begin(); while( ( false == isFind ) & ( anIter != aVectorOfElements->end() ) ) { if( (*anIter)->m_id == anId ) { isFind = true; theReturnedElement = *anIter; } ++anIter; } return( theReturnedElement ); }

---

Mais cela m'oblige à rajouter deux variables internes à la fonction.

Salut,

avec un break

Code:

---

1 2 3 4 5 6

for ( ... ) { if (true) break; }   return ... ;

---

Citation:

---

Envoyé par minnesota

Salut,

avec un break

Code:

---

1 2 3 4 5 6

for ( ... ) { if (true) break; }   return ... ;

---

---

Son erreur ne vient pas du fait qu'il ai un return dans sa boucle for mais qu'il n'ai pas de return après sa boucle for. En effet, ta fonction doit forcément retourner quelque chose vu qu'elle n'est pas de type 'void', or que se passe-t-il si ta condition du 'if' n'est jamais vérifiée et que tu sors de ton for ? Tu ne retourne alors rien.

Code:

---

1 2 3 4 5 6 7 8 9 10 11

Element * searchElement( std::string anId , std::vector< Element * > * aVectorOfElements ) { std::vector< Element * >::iterator anIter = aVectorOfElements->begin(); for( ; anIter != aVectorOfElements->end() ; ++anIter ) if( (*anIter)->m_id == anId ) return *anIter; return NULL; } //tu pouvais aussi supprimer les accolades inutiles

---

Faire

Code:

---

1 2 3 4 5 6 7

for ( ; anIter != aVectorOfElements->end() ; ++anIter ) { if ( (*anIter)->m_id == anId ) break; }   return (*anIter);

---

Est encore pire. Si tu sors de ta boucle, tu va effectuer l'opération *anIter qui correspondra à *aVectorOfElements->end() = opération illégale




EDIT : je n'avais pas vu :

Code:

---

1 2	Element * theReturnedElement = NULL;

---

Mais tu te complique la vie bien inutilement, ton code devient un peu plus lent et tu perd aussi niveau lisibilité.




EDIT2 : J'oubliais aussi une autre erreur :

Code:

---

1 2	( false == isFind ) & ( anIter != aVectorOfElements->end() )

---

C'est :

Code:

---

1 2	( false == isFind ) && ( anIter != aVectorOfElements->end() )

---

Et tu pouvais même utiliser un for :

Code:

---

1 2	for(initialisation; condition1 && condition2 ; ... )

---

Le tout est de ne pas trop mettre de condition de le for, 2 maximum je dirais pour rester lisible.

Salut,

Il faut comprendre que le compilateur est un brave soldat qui fait ce qu'on lui dit, sans se poser de question...

Aussi, si tu lui dit qu'une fonction doit renvoyer quelque chose, il va, tout simplement, s'étonner si, en fin de fonction, il n'y a pas une instruction de retour ;)

Tu as donc deux solutions :

1. Soit tu es sur que, quoi qu'il arrive, tu trouvera effectivement l'élément dans ton tableau (mais peux tu seulement le certifier en toutes cirocnstances :question:), et, dans ce cas, tu peux tout simplement ignorer cet avertissement
2. Soit, et c'est sans doute mieux, renvoie une valeur clairement identifiée comme une valeur invalide (NULL, ou nullptr en C++11 ) si l'élément n'a pas été trouvé dans le tableau.

Il n'y a rien qui t'interdise d'avoir plusieurs instructions return dans ta fonction, mais, si elle doit renvoyer une valeur (non void ;)), la dernière instruction doit être un return :D

Pour ne pas avoir à parcourir l'ensemble du tableau, tu peux donc parfaitement renvoyer la valeur dés que tu la trouves, mais, il est "de bon ton" de prévoir le cas où la valeur n'a pas été trouvée (ne serait-ce que parce que tu peux avoir un effet de bord "quelque part" qui peut avoir invalidé ta valeur de recherche ;) )

L'idéal est donc d'avoir un code proche de

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12

Element * searchElement( std::string anId , std::vector< Element * > const & aVectorOfElements ) { std::vector< Element * >::iterator anIter = aVectorOfElements.begin(); for( ; anIter != aVectorOfElements.end() ; ++anIter ) { if( (*anIter)->m_id == anId ) { return( (*anIter) ); } } return NULL; }

---

PS : plutot que de passer le tableau par pointeur comme tu l'as fait, passes le par référence constante, c'est plus facile à manipuler, et cela te permet de respecter plus facilement la const correctness ;)

Merci pour les réponses.

Citation:

---

Envoyé par Neckara

Son erreur ne vient pas du fait qu'il ai un return dans sa boucle for mais qu'il n'ai pas de return après sa boucle for. En effet, ta fonction doit forcément retourner quelque chose vu qu'elle n'est pas de type 'void', or que se passe-t-il si ta condition du 'if' n'est jamais vérifiée et que tu sors de ton for ? Tu ne retourne alors rien.

Code:

---

1 2 3 4 5 6 7 8 9 10 11

Element * searchElement( std::string anId , std::vector< Element * > * aVectorOfElements ) { std::vector< Element * >::iterator anIter = aVectorOfElements->begin(); for( ; anIter != aVectorOfElements->end() ; ++anIter ) if( (*anIter)->m_id == anId ) return *anIter; return NULL; } //tu pouvais aussi supprimer les accolades inutiles

---

Faire

Code:

---

1 2 3 4 5 6 7

for ( ; anIter != aVectorOfElements->end() ; ++anIter ) { if ( (*anIter)->m_id == anId ) break; }   return (*anIter);

---

Est encore pire. Si tu sors de ta boucle, tu va effectuer l'opération *anIter qui correspondra à *aVectorOfElements->end() = opération illégale

---

J'ai des conditions qui supposent que je trouverai toujours un élément vérifiant la condition du if.

Citation:

---

Envoyé par Neckara

EDIT : je n'avais pas vu :

Code:

---

1 2	Element * theReturnedElement = NULL;

---

Mais tu te complique la vie bien inutilement

---

Il s'agissait d'une proposition qui ne me plait pas.

Citation:

---

Envoyé par Neckara

ton code devient un peu plus lent et tu perd aussi niveau lisibilité.

---

Pourquoi ?

Citation:

---

Envoyé par Neckara

EDIT2 : J'oubliais aussi une autre erreur :

Code:

---

1 2	( false == isFind ) & ( anIter != aVectorOfElements->end() )

---

C'est :

Code:

---

1 2	( false == isFind ) && ( anIter != aVectorOfElements->end() )

---

---

Il me semble justement qu'en ne mettant qu'un seul '&', on fait de la manière suivante :
- on vérifie la condition ( false == isFind ) :
- si c'est faux, on sort de la boucle
- sinon on vérifie la condition ( anIter != aVectorOfElements->end() )
alors qu'en mettant deux '&', on fait les deux vérifications en même temps (perte de temps si ( false == isFind ) est faux

Citation:

---

Envoyé par Neckara

Et tu pouvais même utiliser un for :

Code:

---

1 2	for(initialisation; condition1 && condition2 ; ... )

---

Le tout est de ne pas trop mettre de condition de le for, 2 maximum je dirais pour rester lisible.

---

Suivant comment tu indentes et sautes des lignes, plus de 2 conditions peut rester lisible.

Citation:

---

Envoyé par koala01

Salut,

Il faut comprendre que le compilateur est un brave soldat qui fait ce qu'on lui dit, sans se poser de question...

Aussi, si tu lui dit qu'une fonction doit renvoyer quelque chose, il va, tout simplement, s'étonner si, en fin de fonction, il n'y a pas une instruction de retour ;)

Tu as donc deux solutions :

1. Soit tu es sur que, quoi qu'il arrive, tu trouvera effectivement l'élément dans ton tableau (mais peux tu seulement le certifier en toutes cirocnstances :question:), et, dans ce cas, tu peux tout simplement ignorer cet avertissement
2. Soit, et c'est sans doute mieux, renvoie une valeur clairement identifiée comme une valeur invalide (NULL, ou nullptr en C++11 ) si l'élément n'a pas été trouvé dans le tableau.

Il n'y a rien qui t'interdise d'avoir plusieurs instructions return dans ta fonction, mais, si elle doit renvoyer une valeur (non void ;)), la dernière instruction doit être un return :D

Pour ne pas avoir à parcourir l'ensemble du tableau, tu peux donc parfaitement renvoyer la valeur dés que tu la trouves, mais, il est "de bon ton" de prévoir le cas où la valeur n'a pas été trouvée (ne serait-ce que parce que tu peux avoir un effet de bord "quelque part" qui peut avoir invalidé ta valeur de recherche ;) )

L'idéal est donc d'avoir un code proche de

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12

Element * searchElement( std::string anId , std::vector< Element * > const & aVectorOfElements ) { std::vector< Element * >::iterator anIter = aVectorOfElements.begin(); for( ; anIter != aVectorOfElements.end() ; ++anIter ) { if( (*anIter)->m_id == anId ) { return( (*anIter) ); } } return NULL; }

---

PS : plutot que de passer le tableau par pointeur comme tu l'as fait, passes le par référence constante, c'est plus facile à manipuler, et cela te permet de respecter plus facilement la const correctness ;)

---

Merci beaucoup pour vos précisions.
Concernant le passage par pointeur, il ne s'agit pas du code complet. J'ai en fait une classe ayant, entre autre, comme attribut un vecteur d'éléments de type Element ; et donc une méthode (dans cette classe) de recherche d'un élément du vecteur suivant un id :

Code:

---

Element searchElement( std::string anId );

---

Mais je n'ai pas mis tout cela pour ne pas surcharger inutilement ma question.

Citation:

---

Il me semble justement qu'en ne mettant qu'un seul '&', on fait de la manière suivante :
- on vérifie la condition ( false == isFind ) :
- si c'est faux, on sort de la boucle
- sinon on vérifie la condition ( anIter != aVectorOfElements->end() )
alors qu'en mettant deux '&', on fait les deux vérifications en même temps (perte de temps si ( false == isFind ) est faux

---

Faux, '&' est un symbole d'opérateur bit à bit :
000 100 & 001 000 = 000 000 -> false
000 100 && 001 000 -> true

L'opération '&&' doit théoriquement s'arrêter à la première condition fausse (si je ne me trompe pas). Mais dans la pratique ce n'est pas toujours le cas suivant le compilateur utilisé (il faut donc se méfier).
Par contre il me semble bien que les deux évaluation des conditions avec '&&' se fait de gauche à droite et non en même temps.

Il faut par contre se méfier avec les fonctions car là l'ordre d'évaluation des paramètre est indéterminé (se méfier alors quand on a certaines évaluation de paramètre qui provoquent des effets de bords).

Citation:

---

J'ai des conditions qui supposent que je trouverai toujours un élément vérifiant la condition du if.

---

C'est toujours plus joli de ne pas avoir le warning et ça ne coûte pas grand chose. De plus si jamais tes conditions changent ou si tu veux utiliser cette fonction dans le cas où l'élément n'existe pas forcément, tu n'auras rien à retouché.
Sinon il faudrait au minimum mettre un commentaire pour expliquer cette précondition que les personnes qui lisent ton code puisse le savoir.

Citation:

---

Pourquoi ?

---

Ton code devient plus lent car tu va de voir passer par une variable intermédiaire et quand tu trouvera le résultat tu va devoir faire un break en plus. Mais bon, c'est vraiment négligeable. Mais c'est toujours ça de gagné^^
Niveau lisibilité, c'est plus simple de voir directement le return et la valeur retournée qu'un break et un return où il faut rechercher en haut la valeur par défaut si jamais on a pas trouvé l'élément.

Citation:

---

Suivant comment tu indentes et sautes des lignes, plus de 2 conditions peut rester lisible.

---

Déjà si tu as plus de deux conditions, il faut déjà voir si tu n'as pas un soucis d’algorithme. Et parfois, il vaut mieux faire de simple if avec un break qui de faire une condition à rallonge.
Bon après '2 conditions maximum' n'est pas une règle absolue, c'est plus un conseil. Le tout est de regarder au cas par cas.

Citation:

---

Envoyé par Neckara

Faux, '&' est un symbole d'opérateur bit à bit :
000 100 & 001 000 = 000 000 -> false
000 100 && 001 000 -> true

L'opération '&&' doit théoriquement s'arrêter à la première condition fausse (si je ne me trompe pas). Mais dans la pratique ce n'est pas toujours le cas suivant le compilateur utilisé (il faut donc se méfier).
Par contre il me semble bien que les deux évaluation des conditions avec '&&' se fait de gauche à droite et non en même temps.

Il faut par contre se méfier avec les fonctions car là l'ordre d'évaluation des paramètre est indéterminé (se méfier alors quand on a certaines évaluation de paramètre qui provoquent des effets de bords).

---

Autant pour moi alors. Justement je croyais que cette différence permettait d'éviter de potentiels effets de bord.

Citation:

---

Envoyé par Neckara

C'est toujours plus joli de ne pas avoir le warning et ça ne coûte pas grand chose. De plus si jamais tes conditions changent ou si tu veux utiliser cette fonction dans le cas où l'élément n'existe pas forcément, tu n'auras rien à retouché.
Sinon il faudrait au minimum mettre un commentaire pour expliquer cette précondition que les personnes qui lisent ton code puisse le savoir.

---

Le rajout du "return( NULL );" me semble la solution à réaliser.
Le commentaire était déjà inclus.

Citation:

---

Envoyé par Neckara

Déjà si tu as plus de deux conditions, il faut déjà voir si tu n'as pas un soucis d’algorithme. Et parfois, il vaut mieux faire de simple if avec un break qui de faire une condition à rallonge.
Bon après '2 conditions maximum' n'est pas une règle absolue, c'est plus un conseil. Le tout est de regarder au cas par cas.

---

C'est bien ce que j'ai dit.

Merci en tout cas pour vos réponses.

Même si j'ai tendance à lire très très rapidement et à répondre en premier message très très succinctement aux profils "pre inactifs", cf les abimes, j'aurais dû étoffer mon exemple inutile de 23h pour qu'il le soit :mouarf:.

Aussi, définir une valeur de retour par défaut, l'assigner au besoin est une manière non moins élégante qu'une autre de faire les choses proprement (if> assignation> break). Pour ce qui est de l'optimisation prématurée, je dirais qu'optimiser un algorithme c'est une chose, perdre 10 cycles d'horloge sur un programme en phase de dev c'en est une autre.

Bref, quoi qu'il en soit mon -2x2 est mérité (j'hésite entre :aie: :langue: et :calim2:) cependant, pourquoi lui vous lui collez aussi un -2x2 alors qu'il ne fait que poser des questions, bientôt les gens il vent avoir peut d'ouvrir des sujets :aie:

Citation:

---

Envoyé par minnesota

Bref, quoi qu'il en soit mon -2x2 est mérité (j'hésite entre :aie: :langue: et :calim2:) cependant, pourquoi lui vous lui collez aussi un -2x2 alors qu'il ne fait que poser des questions, bientôt les gens il vent avoir peut d'ouvrir des sujets :aie:

---

J'avoue, je suis une des personnes qui t'as mis un vote négatif, sans rancune ?

Pour les votes négatif sur le post du PO, j'avoue ne pas comprendre moi-même.
Certaines fois un vote négatif peut se comprendre quand le PO abuse un peu.
Parfois on trouve des votes bizarres aussi, généralement dans ce cas là je demande la raison de ce vote, voir si il y a pas une erreur dans ce qui a été dit etc...

Dans le post du PO on pourrait justifier le vote négatif par les erreurs qu'il a dit, mais je rejoins minnesota sur le fait que c'est décourageant pour la personne qui pose des questions.
Peut-on vraiment sanctionner négativement une personne qui essaye de comprendre et de s'améliorer même si elle dit des bêtises?
Il vaut mieux dire des bêtises ici et apprendre que de continuer à faire les mêmes erreurs.
C'est une des raisons pour laquelle j'ai contrebalancé avec 3 votes positifs.


Bon après, ceci n'a pas une importance fondamentale et ça n'empêchera pas le PO de dormir^^

On est hs, mais je te réponds par courtoisies vu que tu poses la question.

Aussi, je le savais très bien, mais tu ne risques rien, car ma politique de vote dans la partie technique, depuis très longtemps déjà, c'est de mettre en valeur les postes de qualité (pour faire de jolis cadres verts), donc soit c'est plus, soit c'est rien. Si je suis l'auteur du sujet, je mets systématiquement un +1 à ceux qui ont la bonté de répondre, c'est ma manière de dire merci (du coup, on arrive plus vite aux jolis cadres verts vu que dans la partie technique les votes ne sont pas légion).

Par contre, même si je l'ai déjà fait je me questionne sur l'intérêt de contrebalancer pour contrebalancer, parce que dans le fond ça ne sert pas l'intérêt général du forum, enfin, je trouve. J'ai remarqué aussi qu'il y en a qui mette un +1 sur un post pour annuler leur vote négatif d'un post précédent, alors qu'il est possible d'annuler un vote en appuyant sur le même bouton qui l'a validé. Mais bon, comme on est hs par rapport au sujet, le mieux c'est de soulever le problème dans la section adéquate.

Et puis comme on est dans le hs, tant qu'à faire, j'en profite pour dire que j'ai capturé un moustique affamé gros comme une vache, l'été s'annonce chaud :mouarf:

Bonsoir,
Pour répondre a la question initial je pense que j'aurais ajouté une exception, du style:

Code:

---

throw std::logic_error("Cette ligne ne devrait pas être atteinte");

---

plutot qu'un "return NULL".
Il y as trois raison a cela:
1 - Le relecteur du code sait immédiatement a quoi s'en tenir
2 - Comme le "return", ça fait taire le warning
3 - On as pas besoin(ni même l'idée) de tester la valeur retournée à la sortie de la fonction.

Citation:

---

Envoyé par Nogane

Bonsoir,
Pour répondre a la question initial je pense que j'aurais ajouté une exception, du style:

Code:

---

throw std::logic_error("Cette ligne ne devrait pas être atteinte");

---

plutot qu'un "return NULL".
Il y as trois raison a cela:
1 - Le relecteur du code sait immédiatement a quoi s'en tenir
2 - Comme le "return", ça fait taire le warning
3 - On as pas besoin(ni même l'idée) de tester la valeur retournée à la sortie de la fonction.

---

C'est effectivement une solution, mais elle ne doit être mise en oeuvre que dans le cas où tu sais pertinemment que la recherche doit donner un résultat...

Encore une fois, il faut voir le contexte général car... peux tu vraiment certifier que, quoi qu'il arriive, tu trouveras l'élément recherché :question:

[édit] (oupps mes doigts ont glissé )

Il ne faut pas oublier que l'exception va remonter la pile d'appel jusqu'à tomber sur un bloc try ... catch dont le catch est compatible avec l'exception lancée.

Mais cela sous entend qu'il faut être en mesure de gérer l'exception, de préférence en apportant une solution qui permette d'éviter qu'elle ne se reproduise sous peine de voir ton application planter purement et simplement sur une "unhandled exception" :aie:

Tu me diras sans doute que ce cas de figure n'est pas pire qu'un plantage du au fait que tu aurais essayé de déréférencer NULL, mais elle me semble, sous certaines conditions du moins, peut etre un peu... expéditive ;)

En effet, le fait qu'un élément n'existe pas n'est pas forcément une erreur et n'est pas forcément "exceptionnel" !

Alors, effectivement, l'exception peut être adaptée à certains contexte, sous réserve que l'utilisateur reste conscient qu'elle peut etre lancée (et qu'il veille donc à l'attraper au plus tard au niveau de l'interface graphique, par exemple), mais, dans d'autres contextes, il sera peut etre beaucoup plus logique de simplement vérifier la validité du pointeur retourné ;)

Je suis tout a fait d'accord que choisir throw ou return NULL dépend du contexte. Mais dans le cas présent, le PO est sur que l'objet recherché existe.

Citation:

---

- lorsque je fais une recherche pour anId, je suis sûr qu'il existe bien un élément du vecteur qui à comme attribut m_id cet valeur anId (ce qui suppose en plus que ce vecteur n'est pas vide.

---

Pour moi, le pointeur ne sera pas NULL car ça fait partie des postconditions de la fonction.
Tout ce qui va suivre par donc bien de l'idée que, a par a cause d'un bug, l'objet recherché sera toujours trouvé.
Donc d'un coté on peu lancer une exception, en prenant le risque que ça stop(proprement) le programme si elle n'est pas gérée.
De l'autre coté, on peu retourner NULL... mais réfléchissons au conséquences de ce choix:
Toute les fonctions appelantes vont devoir tester si la valeur retournée et NULL et gérer ce cas. Pour cela, 4 choix possibles:
1 - En retournant un pointeur NULL, qui devrai a son tour être tester et les test de pointeurs NULL vous se propager partout dans le code.
2 - En lançant une exception(dans ce cas autant le faire de suite)
3 - En retournant un code d'erreur, mais beaucoup de gens préfère(souvent a juste titre) utiliser des exceptions plutôt que des codes d’erreurs. En plus le teste de ce code d’erreur va se propager dans tout les fonctions appelantes.
4 - En faisant un assert(false && "bla bla bla"), mais comme pour l'exception, ça aurais pu être fait dans la fonction "searchElement".


Bon après, le plus important n'est pas de trouver la bonne manière de gérer l'erreur, mais de bien documenter la fonction pour que son futur utilisateur sache comment elle réagit en cas d’erreur.

Dans ce cas, il a comme pré-condition qu'il trouvera l'élément.

Mais il fait comment s'il aura, plus tard, besoin de la même fonction mais dont cette pré-condition n'est pas vérifiée?

Il refait la même fonction en retournant cette fois NULL ?

C'est à la fonction appelante de décider que faire si jamais il y a une erreur.

Ce n'est pas très compliqué d'ajouter :

Code:

---

1 2 3 4 5

if(ptr = searchElement(...)) { //traitement de l'erreur }

---

De plus, la post condition dit : si l'élément est sûr d'être trouvé, la fonction ne renverra pas NULL, dans ce cas là, il est peut être inutile de tester la valeur de retour.

Si la fonction appelante peut lancer une exception, mais c'est à elle de la lancer. C'est elle qui doit décider de ce qu'elle doit lancer.


EDIT : c'est comme si on avais la fonction factorielle :

Code:

---

1 2 3 4 5 6 7 8 9 10 11

unsigned int factorielle(unsigned int i) { unsigned int resultat = 1; while(i != 1) { resultat *= i; i--; } return resultat; }

---

On sait qu'on ne l'utilisera qu'avec des nombres strictement positif.
On ne va pas gérer le cas où i =0 et on va lancer une exception à la fin de la fonction ??
Et si un jour on a besoin de faire 0! ?

Citation:

---

Envoyé par Neckara

Dans ce cas, il a comme pré-condition qu'il trouvera l'élément.

Mais il fait comment s'il aura, plus tard, besoin de la même fonction mais dont cette pré-condition n'est pas vérifiée?

---

Si une précondition n'est pas vérifiée, c'est la faute de l'appelant, le code appelé n'a pas à s'en occuper. Il peut (et c'est souhaitable) aider à détecter ce manquement de l'appelant, dans les cas où c'est possible, mais c'est une aide au débogage, pas le fonctionnement requis de la fonction.

Il y a un exemple que j'aime bien, c'est la recherche dichotomique, qui impose en pré-condition que l'étendue recherchée soit triée. Si on impose une validation des pré-conditions, on passe d'une complexité en O(log n) à une complexité en O(n), qui serait celle d'un tri sans dichotomie. Autrement dit, dans ce cas, valider les préconditions rend la fonction totalement inutile...


Donc, soit on veut faire une fonction de recherche générique, utilisable sans précautions spéciales, et qui retourne NULL quand l'élément n'est pas trouvé (et qui impose donc à l'appelant de tester cette valeur).

Soit on veut faire une fonction de recherche d'un élément que l'on sait au préalable exister, que l'on document ainsi (ajout d'une précondition), et dans ce cas là, ce que fait la fonction dans le cas où ce n'est pas le cas, peu me chaut. J'ai tendance à préférer un simple assert, mais n'ai rien contre une exception, et peux même survivre à un plantage en bonne et due forme, voire même au fait de retourner une mauvaise valeur (même si je n'accepterais ce dernier cas que si une validation est vraiment coûteuse).

Je pense qu'il faut vraiment distinguer dans la gestion des erreurs deux cas : Les erreurs de programmation (comme ici) et les "erreurs" qui sont en fait la gestion de cas particuliers (par exemple, gérer la saisie d'une valeur incorrecte par l'utilisateur).

Citation:

---

Envoyé par JolyLoic

Si une précondition n'est pas vérifiée, c'est la faute de l'appelant, le code appelé n'a pas à s'en occuper. Il peut (et c'est souhaitable) aider à détecter ce manquement de l'appelant, dans les cas où c'est possible, mais c'est une aide au débogage, pas le fonctionnement requis de la fonction.

---

C'est pour cela que je lui demandais s'il faut réécrire une fonction quasi identique qui cette fois retourne NULL puisqu'on ne peut pas utiliser la fonction originelle vu que la précondition n'est pas vérifiée.
Ce qui est tout de même du gaspillage.


On a aussi une "post-condition" : si l'élément existe dans la liste, alors la fonction ne renverra jamais NULL. Dans ce cas là si on sait que l'élément existe, est-il vraiment nécessaire de tester la valeur de retour ?

Merci à tous pour vos réponses et remarques.

Comme je l'ai dit, dans tous les cas l'id se trouve dans le tableau, sinon c'est qu'il y a eu une erreur préalable (lors du scan d'un des fichiers de configuration, ou problème d'espace mémoire de la machine ?) et le programme n'a donc pas lieu de fonctionner.
Je retiens la solution consistant à faire un return(NULL), reste à voir s'il faut que je vérifie justement la validité du pointeur dans la fonction appelante. Vos dernières remarques me sont donc utiles.

La fonction ayant comme précondition que l'élément recherché doit être présent, dans ce cas a mon sens elle ne doit pas renvoyer un pointeur mais une référence.

Du coup le type de retour explicite clairement la précondition et le contexte appelant n'a même plus a se poser la question de savoir si il doit ou non tester le cas null.

Par extension la fonction n'ayant plus la possibilité de renvoyé un pointeur null, le cas d'erreur doit être traite autrement, par exemple via une exception.

Citation:

---

Envoyé par Gymble

La fonction ayant comme précondition que l'élément recherché doit être présent, dans ce cas a mon sens elle ne doit pas renvoyer un pointeur mais une référence.

---

Pouvez-vous me justifier pourquoi svp ?

Tu trouvera une partie de la réponse dans la FAQ
http://cpp.developpez.com/faq/cpp/?page=references

Une référence ne pouvant pas être nulle, si la signature de ta fonction renvoie une référence elle annonce clairement la couleur: elle renverra forcement un objet valide. Ce qui correspond a la pre-condition.

Citation:

---

Envoyé par Gymble

Tu trouvera une partie de la réponse dans la FAQ
http://cpp.developpez.com/faq/cpp/?page=references

Une référence ne pouvant pas être nulle, si la signature de ta fonction renvoie une référence elle annonce clairement la couleur: elle renverra forcement un objet valide. Ce qui correspond a la pre-condition.

---

Merci.

Néanmoins, me reste à implémenter cela avec le problème de l'itérateur de recherche : que dois-je renvoyer alors ?

Code:

---

1 2 3 4 5 6 7 8 9 10 11

Element& searchElement( std::vector< Element * >& aVectorOfElements , std::string const anId ) { std::vector< Element * >::iterator anIter = aVectorOfElements.begin(); for( ; anIter != aVectorOfElements.end() ; ++anIter ) { if( (*anIter)->m_id == anId ) { return( *anIter ); } } }

---

le passage

Code:

---

*anIter

---

provoque une erreur de compilation (que je comprends bien) mais je ne vois pas trop quoi mettre :calim2:...

anIter est une sorte de pointeur (pour faire simple) vers un élément de ton vecteur.

*anIter est donc un élément de ton vecteur, c'est à dire un Element *.
Si tu veux un élément il faut utiliser **anIter

Code:

---

1 2	return **anIter;

---

Citation:

---

Envoyé par Neckara

anIter est une sorte de pointeur (pour faire simple) vers un élément de ton vecteur.

*anIter est donc un élément de ton vecteur, c'est à dire un Element *.
Si tu veux un élément il faut utiliser **anIter

Code:

---

1 2	return **anIter;

---

---

Après coup, c'est ce que j'ai compris oui :oops:.
Néanmoins, cela ne règle pas le problème de warning initial :(

Une exception après le for règle le problème du warning.

Edit : Comme cela a été dis plus haut, par exemple:

Code:

---

1 2 3 4 5 6 7 8 9

Element& searchElement( std::vector< Element * >& aVectorOfElements , std::string const &anId ) { std::vector< Element * >::iterator anIter = aVectorOfElements.begin(); for( ; anIter != aVectorOfElements.end() ; ++anIter ) if( (*anIter)->m_id == anId ) return( **anIter );   throw std::logic_error("Cette ligne ne devrait pas être atteinte"); }

---

Il faut que tu jette une exception à la fin de ta fonction :

Code:

---

1 2 3 4 5 6

Type exception; throw(exception);   exemple : throw("Erreur, précondition : 'l'élément doit être dans l'ensemble des Élément' non respecté!"); //on jette un char *

---

Merci pour vos réponses. Je vais donc regarder comment implémenter des exceptions.

En fait, généralement, on va créer un hiérarchie de classes correspondant aux différentes exceptions qui pourront être lancées, car la réaction lorsqu'elle est attrapée peut varier en fonction de ce qui l'a provoquer...

Tu pourrais très bien avoir une classe Exception proche de

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12

class Exception : public std::runitme_error // runtime_error est l'exception standard pour dire qu'une erreur est survenue à l'exécution { public: Exception(std::string const & what, std::string const & more): std::runtime_error(what), more_(more){} /* on peut donner encore d'avantage d'information pour préciser   * le contexte, si on décide d'afficher l'erreur)   */ std::string const & more() const{return more_;} private: std::string more_; };

---

Pour un élément non trouvé, la classe dérivée pourrait ressembler à

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12

class ElementNotFound : public Exception { static std::string stringizer(std::string id) { std::string temp("L'element "); temp.append(id) .append("n'a pas ete trouve"); return temp; } public: ElementNotFound(std::string const & id):Exception("ElementNotFound",stringizer(id)){} };

---

et le code d'appel de searchElement serait proche de

Code:

---

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

void foo(/* ... */) { try { /* ... */ Element & elem = searchElement(tab, id); } /* l'exception qui nous intéresse */ catch(ElementNotFound &) { // ce qu'il faut faire } /* au cas où la logique aurait fait lancer une autre exception à nous */ catch(Exception & ) { // ce qu'il faut faire } /* pour les exceptions runtime éventuellement lancée par STL */ catch(std::runtime_exception &) { // ce qu'il faut faire } /* pour toutes les autres exceptions */ catch(... ) { std::cout<<"une exception non envisagee a ete lancee, l'application va quitter"; exit(1); } }

---

Merci pour cette indication, je regarde cela.

std::find ca ne va pas ?

Si il utilisait un std::map avec pour clé m_id, oui on aurait pu utiliser la méthode find, mais dans ce cas là, tu ne peux pas utiliser un find.

Il faudrait redéfinir l'opérateur d'égalité pour pouvoir utiliser find.

Sans redéfinir l'opérateur d'égalité, on peut utiliser un foncteur approprié.

Le post part vraiment sur plein de sujets, dur à suivre !

Citation:

---

Envoyé par Nogane

Bonsoir,
Pour répondre a la question initial je pense que j'aurais ajouté une exception, du style:

Code:

---

throw std::logic_error("Cette ligne ne devrait pas être atteinte");

---

plutot qu'un "return NULL".
Il y as trois raison a cela:
1 - Le relecteur du code sait immédiatement a quoi s'en tenir
2 - Comme le "return", ça fait taire le warning
3 - On as pas besoin(ni même l'idée) de tester la valeur retournée à la sortie de la fonction.

---

Personnellement moi ici je met un assert qui fait sauter l'exécution direct. Pour moi l'exception doit être utilisée dans le cas où l'erreur doit être gérée dans un cas d'utilisation possible du programme. Si la précondition n'est pas respectée ici, c'est sûrement la faute d'un développeur et pas du système ou de l'utilisateur. Donc assert et correction obligatoire du code incriminé.

Bien sûr cela dépend du contexte (ma position n'est peut être pas valable pour du code embarqué critique).

Citation:

---

Envoyé par jblecanard

Le post part vraiment sur plein de sujets, dur à suivre !

Personnellement moi ici je met un assert qui fait sauter l'exécution direct. Pour moi l'exception doit être utilisée dans le cas où l'erreur doit être gérée dans un cas d'utilisation possible du programme. Si la précondition n'est pas respectée ici, c'est sûrement la faute d'un développeur et pas du système ou de l'utilisateur. Donc assert et correction obligatoire du code incriminé.

Bien sûr cela dépend du contexte (ma position n'est peut être pas valable pour du code embarqué critique).

---

Bonjour et merci à tous pour vos réponses.
En fait, la seule fois où l'élément n'existe pas serait (je pense) dans le cas d'une allocation mémoire impossible d'un élément dans le vecteur (pas assez de mémoire). Le vecteur d'éléments est construit une fois pour toute au début de l'exécution du programme et ensuite durant toute l'exécution les appels à cette fonction de recherche ne se font que pour des ID existants (c'est sûr).

Citation:

---

Envoyé par Bousk

Sans redéfinir l'opérateur d'égalité, on peut utiliser un foncteur approprié.

---

+++
En C++11, ça devient une ligne :

Code:

---

1 2 3 4

Element& searchElement( std::vector< Element * >& aVectorOfElements , std::string const &anId ) { return **std::find_if(std::begin(aVectorOfElements),std::end(aVectorOfElements),[&anId](Element const*e_){return e_->m_id==anId;}); }

---

Citation:

---

Envoyé par 3DArchi

+++
En C++11, ça devient une ligne :

Code:

---

1 2 3 4

Element& searchElement( std::vector< Element * >& aVectorOfElements , std::string const &anId ) { return **std::find_if(std::begin(aVectorOfElements),std::end(aVectorOfElements),[&anId](Element const*e_){return e_->m_id==anId;}); }

---

---

Je vais regarder cela plus en détail (notamment ce qu'est un foncteur).

Citation:

---

Envoyé par jblecanard

Le post part vraiment sur plein de sujets, dur à suivre !



Personnellement moi ici je met un assert qui fait sauter l'exécution direct. Pour moi l'exception doit être utilisée dans le cas où l'erreur doit être gérée dans un cas d'utilisation possible du programme. Si la précondition n'est pas respectée ici, c'est sûrement la faute d'un développeur et pas du système ou de l'utilisateur. Donc assert et correction obligatoire du code incriminé.

Bien sûr cela dépend du contexte (ma position n'est peut être pas valable pour du code embarqué critique).

---

Je n'ai absolument rien contre l'assert, mais rappelons que la question de départ était de faire taire le warning, ce que ne fera pas l'assert. Biensur, on peut cumuler assert et throw.
Rappelons quand même que l'exception logic_error doit signaler une erreur dans le code, exactement comme l'assert, et n'est donc pas si éloigné que ça de l'assert dans sa signification(même si les conséquence sont différentes).

Citation:

---

Envoyé par jblecanard

Personnellement moi ici je met un assert qui fait sauter l'exécution direct. Pour moi l'exception doit être utilisée dans le cas où l'erreur doit être gérée dans un cas d'utilisation possible du programme. Si la précondition n'est pas respectée ici, c'est sûrement la faute d'un développeur et pas du système ou de l'utilisateur. Donc assert et correction obligatoire du code incriminé.

Bien sûr cela dépend du contexte (ma position n'est peut être pas valable pour du code embarqué critique).

---

Je vois, personnellement deux gros problèmes à l'utilisation des seules assertions.

Le premier (que j'admets beaucoup plus volontiers pour les exceptions) est que ce n'est pas parce que, en période de tests (qu'il soient unitaires ou basés sur des scénarios de manipulation réelle) tu n'as pas une assertion que cela veut, d'office, dire que ton code est correct...

Cela veut "simplement" dire que, pour l'instant, tu n'as pas encore trouvé le moyen de mettre ton système dans un état dans lequel l'assertion surviendrait ;)

Je suis le premier à reconnaitre la vertu des tests unitaires et des tests d'utilisation courante, mais ce n'est, de manière générale, pas parce que ton système les passe haut la main qu'il n'y a strictement aucun problème, cela veut dire, tout simplement, que "en l'état", tu n'as pas encore trouvé le moyen de mettre le système en défaut ;)

Le deuxième que je trouve aux assertions tient à leur nature même qui veut qu'elles ne soient actives qu'en mode débug.

Cela signifie que, dés que tu présentes une version release, les assertions sont, purement et simplement, supprimées de l'exécutable et que, bien souvent on va avoir un "crash inexpliqué" qui survient en utilisation réelle (mais donc en version release) justement parce que la seule protection était... un assert, certes judicieux, mais qui n'est pas actif dans la release :aie:

Et c'est quelqu'un qui travaille sur un projet qui contient plus d'assertions que de fonction qui écrit ces lignes ;)

Citation:

---

Envoyé par koala01

Je vois, personnellement deux gros problèmes à l'utilisation des seules assertions

---

Ne me faites pas dire ce que je n'ai pas dis :mrgreen:
j'ai dit que dans ce cas précis, je met une assertion, car c'est une zone de code qui n'est pas censée être exécutée, et que le système ne doit pas être dans cet état et que s'il s'y retrouve, je dois en être informé aussi tôt que possible et de la manière la plus brutale qui soit, en développement.

De toute évidence, les assertions seules ne suffisent pas comme métrique de qualité et de fiabilité, là dessus (et sur le reste de ta réponse), je suis absolument d'accord ! D'ailleurs, rien n'empêche de coder une assertion brutale en dev qui se signale de manière plus douce en release. On ne peut pas rattraper proprement tous les cas d'erreurs qui proviennent d'états dans lequel le système n'est pas censé se trouver, sinon on finit jamais le dev :(