Discussion :

[r0d]

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 :

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

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.

[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.

[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.

[r0d]

hum oui... 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?

[Jean-Marc.Bourguet]

hum oui... 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?

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

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

Qu'entends-tu par SESE?

Single Entry, Single Exit.

[Luc Hermitte]

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 :

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

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;
}

[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é):

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
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?

[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 :

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

1
2
3
4
int iError = 0;
while (iError == 0)
    iError = pipoFunc1(k);
return iError;

...disons que c'est déjà un peu mieux.

[Jean-Marc.Bourguet]

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

Code :

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

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 :

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

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;
  }
}

[Jean-Marc.Bourguet]

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.

[r0d]

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

[Jean-Marc.Bourguet]

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.