discution sur la gestion des erreurs au niveau d'une méthode

Version imprimable

Bonjour à tous,

je me pose tout un tas question sur la gestion des erreurs. Je suis curieux d'avoir votre avis là-dessus.

Prenons un exemple précis: la fonction suivante
Code:

1 2 3 4 5 6 7 8 9 10 11 12 int PipoClass1::pipoFunc1(int iSize) { // cette fonction retourne un int qui correspond à un code d'erreur. // S'il n'y a pas d'erreur, elle retourne 0 int iErrorCode = 0; //code erreur int * buffer = new int[iSize]; //j'alloue un buffer de int FILE* file = fopen("C:/toto.txt", "r"); //j'ouvre un fichier bool b = pipoFunc2(buffer, iSize); //j'appelle une fonction qui me retourne false en cas d'erreur return iErrorCode; }
Vous remarquerez qu'il manque au moins 2 choses: la gestion des erreurs, et la libération de la mémoire (buffer et file). Je me demande, ici, quelle elle la meilleure façon de gérer les erreurs.

Les solutions que je retrouve habituellement sont:
-> le goto:
en cas d'erreur, on fait un goto LabelError par exemple, et dans le LabelError, on libère la mémoire, on ferme le fichier, etc... Dans ce cas, on n'a qu'un seul return.
-> la gestion au fur et à mesure:
A chaque ligne de code, on vérifie s'il y a une erreur, et on gère en fonction, avec un return à chaque fois.
-> la (fausse) boucle while (0) et les breaks:
on commence la fonction par un while (0), et lorsqu'on tombre sur une erreur, on fait un break. Et on libère la mémoire après le bloc while. Je pense que ça reviens exactement au même que de faire un goto, mais je ne suis pas sûr.

D'aprés vous, quelle est la meilleure façon de procéder? Existe-t-il de meilleures solutions?

Autre chose: j'entends parfois dire qu'il ne faut qu'un seul point de sortie dans une fonction. C'est à dire qu'un seul return. Est-ce exact? Pourquoi?

Merci pour vos suggestions.

10/11/2006, 14h54
bolhrak

Ce mécanisme de gestion c'est du C, pas du C++. EN C++ tu as un mécanisme pour gérer les erreurs : les exceptions.

Pour la libération des ressources, utiliser le principe d'acquisition de ressource par initialisation. Lorsqu'une exception est lancée tu sors de la portée courante donc toutes les ressources sont libérées. Et même si tu n'utilises pas les exceptions, à la sortie de ta fonction les ressources seront de toute manière libérées.
10/11/2006, 14h56
Jean-Marc.Bourguet

Tu oublies comme solution: arreter de faire du C et faire du C++ (RAII pour gerer les resources et dans le cas present std::ifstream et std::vector).

Quand au SESE, c'est un peu une affaire de gout. Il y a des regles de codages qui l'impose. Je ne suis pas un integriste de la chose, mais j'aime bien regarder s'il y a moyen de mettre le code sous cette forme; le resultat est assez souvent plus clair mais on trouve des exceptions -- en particulier dans ce qui est ton sujet ici: le traitement des erreurs.

L'idee theorique derriere, c'est qu'il est plus facile de raisonner au sujet de ce genre de code.
10/11/2006, 15h41
r0d

hum oui... :oops: Je travaille actuellement sur du code qui n'est pas de moi et qui ressemble plus à du C qu'à du C++, du coup je suis un peu perturbé.

Mais le problème est le même. A savoir: si l'on a une méthode un peu grosse qui peut engendrer des erreurs à plusieurs endroits. Quelles sont les bonnes méthodes pour gérer les erreurs? Un goto n'est pas complètement hérétique en C++?

Qu'entends-tu par SESE?
10/11/2006, 15h46
Jean-Marc.Bourguet

Citation:

Envoyé par r0d

hum oui... :oops: Je travaille actuellement sur du code qui n'est pas de moi et qui ressemble plus à du C qu'à du C++, du coup je suis un peu perturbé.

Mais le problème est le même. A savoir: si l'on a une méthode un peu grosse qui peut engendrer des erreurs à plusieurs endroits. Quelles sont les bonnes méthodes pour gérer les erreurs?

Modifier localement le code pour en faire du vrai C++ meme si tu ne reecris pas tout le fichier?

Citation:

Un goto n'est pas complètement hérétique en C++?

C'est toujours une possibilite. Mais bon...

Citation:

Qu'entends-tu par SESE?

Single Entry, Single Exit.

Citation:

Envoyé par r0d

Mais le problème est le même. A savoir: si l'on a une méthode un peu grosse qui peut engendrer des erreurs à plusieurs endroits. Quelles sont les bonnes méthodes pour gérer les erreurs? Un goto n'est pas complètement hérétique en C++?

Le RAII est ton précieux ami si tu veux un code robuste et simple à maintenir.
Après, retours brutaux ou exceptions, c'est pratiquement du pareil au même -- à voir au cas par cas.

Personnellement, je suis limite un fanatique anti-SESE en C++. Surtout quand le SESE concerne des boucles qui passent de 3 à 7 lignes dans une fonction qui en fait plus de 70.
J'y voyais surtout un intérêt dans les langages comme le C où la gestion des ressources ne peut pas être automatisée, et où il n'y a pas d'exceptions -- étrangement l'argument qui revient le plus souvent est celui esthétique de comprehénsion du code et non celui de la collection des ressources, ce qui ne me rassure guère.

Le C++ a
- le RAII qui élimine le besoin de tout collecter explicitement à la fin des fonctions (fini les "goto ERROR"!),
- des exceptions qui font qu'il est illusoire de pouvoir avoir un code SESE simple en toutes circonstances.

Et quand la règle du nombre de lignes réduit (que je considère bien plus importante), est respectée, alors je trouve que le SESE n'apporte plus grand chose en termes de compréhension du code.
J'ai limite tendance à percevoir le SESE comme une excuse pour violer la règle du nombre de lignes réduit (*). D'où mon mépris pour l'exigence de SESE en C++ -- que l'on trouve dans certaines règles qualités qui ne parlent pas de la gestion des ressources et qui en sont restées à la vision d'il y a 10 ans des exceptions.

(*) Il faut dire que quand j'entends "C'est pénible ces règles qui empêchent les fonctions d'avoir plus de 150 lignes." et "Ah non! un seul return par fonction ; pas de break; pas de continue" d'une même bouche, ça n'aide pas à avoir une bonne image du SESE. OK, j'exagère. Mais si peu :(

Sinon, je serais du genre à refactorer ton bout de code en :
Code:

1 2 3 4 5 6 7 8 9 10 int PipoClass1::pipoFunc1(int iSize) { // cette fonction retourne un int qui correspond à un code d'erreur. // S'il n'y a pas d'erreur, elle retourne 0 std::ifstream file("c:/toto.txt"); if (!file) return -1; // @pre fichier dispo std::vector<int> buffer(iSize); const bool b = pipoFunc2(file, &buffer[0], buffer.size()); if (!b) return -2; // @post pipoFunc2 doit s'exécuter return 0; }

Merci beaucoup pour ce point de vue.
Et que pensez-vous de ceci (vu récemment dans du code sur lequel j'ai travaillé):
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 int PipoClass1::pipoFunc1(int iSize) { // cette fonction retourne un int qui correspond à un code d'erreur. // S'il n'y a pas d'erreur, elle retourne 0 int iRet = 0; //valeur de retour while(0) { std::ifstream file("c:/toto.txt"); if (!file) { iRet = -1; break; } std::vector<int> buffer(iSize); const bool b = pipoFunc2(file, &buffer[0], buffer.size()); if (!b) { iRet = -2; break; } } return iRet; }
Je pose la question vraiment sans a-priori car je n'ai jamais eu l'occasion de discuter de tout ça sérieusement. Le code ci-dessus a l'avantage de respecter le SESE. En revanche, il fait presque deux fois plus de lignes, et ça me semble être un peu du "bidouillage". Mais qu'en pensez-vous?

11/11/2006, 00h35
Luc Hermitte
Un exemple typique du ridicule du SESE quand il faut gérer des possibles erreurs.
Je ne trouve franchement pas que l'on gagne en lecture.
En revanche, en gardant la pipoFunc1 initiale, et en définissant pipoFunc0 comme
Code:

1 2 3 4 int iError = 0; while (iError == 0) iError = pipoFunc1(k); return iError;
...disons que c'est déjà un peu mieux.

Citation:

Envoyé par Luc Hermitte

Sinon, je serais du genre à refactorer ton bout de code en :

Code:

1
2
3
4
5
6
7
8
9
10
int PipoClass1::pipoFunc1(int iSize) {
    // cette fonction retourne un int qui correspond à un code d'erreur.
    // S'il n'y a pas d'erreur, elle retourne 0
    std::ifstream file("c:/toto.txt");
    if (!file) return -1; // @pre fichier dispo
    std::vector<int> buffer(iSize);
    const  bool b = pipoFunc2(file, &buffer[0], buffer.size());
    if (!b) return -2; // @post pipoFunc2 doit s'exécuter
    return 0;
}

Personnellement, je l'écrirais comme ceci.

Code:

1
2
3
4
5
6
7
8
9
10
int PipoClass1::pipoFunc1(int iSize)
{
  std::ifstream file("c:/toto.txt");
  if (!file) {
    return -1;
  } else {
    std::vector<int> buffer(iSize);
    return pipoFunc2(file, &buffer[0], buffer.size()) ? 0 : -2;
  }
}

11/11/2006, 08h36
Jean-Marc.Bourguet

Citation:

Envoyé par r0d

Merci beaucoup pour ce point de vue.
Et que pensez-vous de ceci (vu récemment dans du code sur lequel j'ai travaillé):
[snip l'horreur]
Je pose la question vraiment sans a-priori car je n'ai jamais eu l'occasion de discuter de tout ça sérieusement. Le code ci-dessus a l'avantage de respecter le SESE. En revanche, il fait presque deux fois plus de lignes, et ça me semble être un peu du "bidouillage". Mais qu'en pensez-vous?

C'est une horreur. Si tu veux du SSE, tu prends ma proposition et tu remplaces les return par des assignations.
11/11/2006, 10h12
r0d

Citation:

Envoyé par Jean-Marc.Bourguet

C'est une horreur. Si tu veux du SSE, tu prends ma proposition et tu remplaces les return par des assignations.

Oui mais il y a une chose qui me gène un peu dans cette façon de développer, c'est que l'on peut se retrouver avec tout un tas de tests imbriqués, et ça peut devenir difficile à lire. Avec, par exemple, du code tellement indenté que la ligne commence par 10 tabulations.

Je chipotte, j'en suis conscient, mais j'aime bien pousser les discussions jusqu'au bout :D
11/11/2006, 12h20
Jean-Marc.Bourguet

Citation:

Envoyé par r0d

Oui mais il y a une chose qui me gène un peu dans cette façon de développer, c'est que l'on peut se retrouver avec tout un tas de tests imbriqués, et ça peut devenir difficile à lire. Avec, par exemple, du code tellement indenté que la ligne commence par 10 tabulations.

D'une part, à ce niveau là, je me demande si une ou plusieurs fonctions en plus ne sont pas nécessaires.

D'autre part, il est des cas où je ne vais pas mettre les else quand la branche "then" se termine par return. Je ne suis pas intégriste d'une forme donnée, je cherche la forme qui exprime le mieux le sens que je cherche à donner.