Discussion :

[r0d]

Bonjour,

j'ai commencé le dev en entreprise avec la MFC. J'ai mis longtemps à m'en remettre et ré-apprendre à coder correctement en c++, mais j'en garde des séquelles. Notemment cet "idiome du GetLastError" que j'utilise toujours, et même de plus en plus fréquemment.

Ce que j'entends par "idiome du GetLastError", c'est une chaîne de carractère en membre privé d'une classe, avec un accesseur, dans laquelle on stocke un message d'erreur lorsqu'une erreur est survenue lors d'un traitement au sein de cette classe. L'idée est qu'ainsi les fonctions membres de cette classe ne renvoient qu'un booléen (true si ok, false si une erreur est survenue), et lorsque l'utilisateur de cette classe veut savoir ce qu'il s'est passé, il appelle cette fonction GetLastError().

Un peu de code pour illustrer:

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
class MaClasse
{
public:
    bool FaitQuelqueChose(); // return true si ok, false si erreur
    inline const std::string & GetLastError() const { return last_error_; }
private:
    mutable std::string last_error_; //mutable car on veut pouvoir gérer l'erreur même dans les fonctions constantes
};
 
int main()
{
   MaClasse ma_classe;
   if ( !ma_classe.FaitQuelqueChose() )
   {
      // traitement de l'erreur en utilisant ma_classe.GetLastError();
   }
   //...
}

Le truc c'est que je vois beaucoup d'avantages, mais pas d'inconvénients. Les avantages sont:
. indépendance de cette classe avec le système de log de l'application (qui peut être complexe parfois)
. simplification de l'interface: les fonctions susceptibles de générer des erreurs ne renvoient qu'un simple booleen.
. avec cette technique, pas besoin de remonter les exceptions.
. simplification d'utilisation de la classe : l'utilisateur de cette classe ne s'occupe des erreurs que s'il en a besoin.

Mais comme je ne vois pas beaucoup d'utilisation de cet idiome (à part dans la MFC), je suppose qu'il doit y avoir de nombreux inconvénients. Savez-vous quels sont-ils?

[Davidbrcz]

Sans réflexion approfondie, voilà ce que je réponds/

. simplification de l'interface: les fonctions susceptibles de générer des erreurs ne renvoient qu'un simple booleen.

1) impossibilité à ta fonction de retourner un objet. Ca peut rendre bancale une architecture

2) Tu n'as pas forcément la "vraie" erreur dans le cas d'appel en cascade.
Ex

Code :

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

1
2
3
4
5
6
7
8
9
10
class Foo
{
std::string erreur;
public:
bool f1(){ if(machin){erreur="machin dans f1"; else return true;}
bool f2() ( //f1 fait un truc mais tu ne vois pas l'erreur tout de suite
if(bidule) erreur="erreur dans f2"; //bidule depend de l'action de f1
} 
//si tu fais un appel à f2 depuis l'extérieur, tu auras la valeur d'erreur de  f2.
};

3) Obligation de tester tout les retours, niveau lourdeur et redondance de code c'est sympa ....

. indépendance de cette classe avec le système de log de l'application (qui peut être complexe parfois)

Enfin ton loggeur devra avoir accès à la classe pour récupérer la valeur de la chaîne d'erreur. Avec ta technique, ta gestion d'erreur empiète sur le rôle de la classe alors que ces deux notions sont normalement orthogonales.

. avec cette technique, pas besoin de remonter les exceptions.

Les exceptions permettent une gestion non nominale de la progression de façon automatisée. Avec ton code, quand tu détectes une erreur, bah tu fais quoi ?

simplification d'utilisation de la classe : l'utilisateur de cette classe ne s'occupe des erreurs que s'il en a besoin.

Une erreur devrait toujours être signalée et traitée

[metagoto]

Je préfère l'idiome dit du try catch switch GetLastError return ERROR_CODE

Code :

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

1
2
3
4
5
6
7
try {
  //
}
catch (...) {
  switch (GetLastError()) {
    case TYPE_ERREUR_N1: return ERROR_FAIL;
    default: throw;

[nikko34]

C'est du errno local à chaque classe? mais j'imagine que ça a les mêmes problèmes, en cas de multithread aussi c'est pas la fête non?

Perso j'ai pas de problème à utiliser des exceptions/ messages d'erreurs dans les logs/à l'écran.

[r0d]

C'est du errno local à chaque classe? mais j'imagine que ça a les mêmes problèmes, en cas de multithread aussi c'est pas la fête non?

Quels peuvent être les problèmes en multi-thread?

[r0d]

1) impossibilité à ta fonction de retourner un objet. Ca peut rendre bancale une architecture

Si je veux retourner un objet, j'utilise un paramètre passé par référence. D'ailleurs en règle générale c'est pas terrible qu'une fonction retourne directement un objet car ça fait une copie.

2) Tu n'as pas forcément la "vraie" erreur dans le cas d'appel en cascade.

Oui effectivement, ça c'est un vrai problème. C'est d'ailleurs explicite dans le nom de la fonction: GetLastError. on récupère la dernière erreur.

3) Obligation de tester tout les retours, niveau lourdeur et redondance de code c'est sympa ....

Ben non pas forcément. Seules les fonctions susceptible de générer des erreurs auront leur résultat testé. Et ça, je ne vois pas comment l'éviter de toutes façon. A moins de faire un gros bloc try, mais c'est pas terrible.

Enfin ton loggeur devra avoir accès à la classe pour récupérer la valeur de la chaîne d'erreur. Avec ta technique, ta gestion d'erreur empiète sur le rôle de la classe alors que ces deux notions sont normalement orthogonales.

Ben de toutes façons, c'est soit la classe qui accède au logger, soit l'inverse. Je ne vois pas en quoi l'un est meilleur que l'autre.

Les exceptions permettent une gestion non nominale de la progression de façon automatisée. Avec ton code, quand tu détectes une erreur, bah tu fais quoi ?

Comprends pô

Une erreur devrait toujours être signalée et traitée

Mmhh... c'est la première fois que j'entends cela. C'est peut-être ce qu'on enseigne en théorie, mais dans la pratique ce n'est tout simplement pas possible, et rarement recommandable. Dès qu'une applicatin commence à être un peu complexe, il y a tout un tas d'erreur qu'il est totalement inutile de gérer. Par exemple je sais pas, on recherche un objet dans un conteneur dans le but de le supprimer et on ne le trouve pas alors qu'il devrait y être. Franchement, 9 fois sur 10, on ne fait rien dans ce cas là.
Après ça dépend de ce qu'on appelle une erreur.

[camboui]

Ces fonctions qui renvoient un code d'erreur incitent le programmeur à tester systématiquement ce code d'erreur. Bonjour la lisibilité du code bourré de "if" après chaque appel de fonction. Et ça ne le rend pas plus fiable pour autant. Que du contraire, le programmeur a la responsabilité de prendre une décision pour gérer l'erreur, comment garantir qu'il fera le bon choix ?
Et d'ailleurs dans le "else" il fera quoi ? Dire boujour sur la console pour expliquer que "le disque est bourré à mort", ou envoyer une exception pour dire la même chose dans le main juste avant la sortie du prg

Les fonctions de l'API Windows par exemple renvoient un code d'erreur de manière quasi systématique. Chaque fois que je regarde les exemples de code pour illustrer une utilisation, les trois-quarts concerne la gestion du code retour...
OK pour admettre qu'une API se débarasse de cette responsabilité de gérer les erreurs. Mais si à chaque niveau de couche logicielle on renvoit la responsabilité à la couche supérieur, il y aura un lampiste au bout du code qui va souffrir parce qu'on lui impose un rôle qui n'est pas le sien. Et tout ça rien que pour gérer des erreurs, c'est à dire des situations anormales et rares, exceptionnelles devrait-on dire. Il n'y a pas le mot "exception" là-dedans ?

Je préfère une approche "transactionnelle" de la gestion d'erreur. Les exceptions y aide grandemment, et sont même faites pour cela en C++ il me semble.

[Invité]

Mais comme je ne vois pas beaucoup d'utilisation de cet idiome (à part dans la MFC), je suppose qu'il doit y avoir de nombreux inconvénients. Savez-vous quels sont-ils?

J'ai l'impression que ce genre d'approche est limitée à des classes ayant un petit nombre d'instances, et une durée de vie assez longue. Dans les autres cas :

Pour de petites classes, il va y avoir un surcout lié à la création d'une chaine pour chaque instance.

Si ces classes ont vocation à être copiées (si par exemple tu les utilises dans des conteneurs STL, c'est copie garantie dans ces cas là), il va falloir décider si la dernière erreur doit ou non être copiée... Normalement, la réponse est probablement non, ce qui signifie un constructeur de copie et un opérateur copie à faire à la main. (tu peux peut être éviter cela en encapsulant ton message d'erreur dans un type qui gère cela tout seul, mais bon...)

Plus généralement, il faut absolument "consommer l'erreur" dès qu'elle est détectée... Les problèmes liés à l'éventuelle copie de ton instance font qu'un "vieux" LastError peut être très difficile à interpréter.

Enfin, la gestion des erreurs se trouve alors au niveau de l'instance de chaque classe. Ca ne marchera bien que pour des erreurs "propres à l'instance": n'a pas pu s'initialiser, n'a pas réussi à faire telle action, etc... pour des messages d'erreur provenant de l'extérieur de l'instance (réseau en panne, donnée en entrée erronnée), ca peut ne pas convenir.

En résumé, je crois que c'est une bonne approche pour des classes assez statiques, et qui fournissent des "services" (comme les composants d'une API d'OS en fait...), mais elle ne peut servir de "méthode standard" de gestion d'erreur.

Francois

[Alp]

Quels peuvent être les problèmes en multi-thread?

Si plusieurs threads utilisent la classe qui a le get last error là, un thread peut appeler GetLastError et récupérer l'erreur liée à un autre thread parce que l'autre thread entre temps a utilisé ton objet et le get last error a été modifié... ce genre de choses. Donc à moins que tu mette un gros lock sur tuot le code, ce qui va ralentir abominablement l'exécution et bloquer pas mal de threads...

Je préfère encore une approche fortement orientée exceptions ou un truc à base de boost::variant<résultat_normal, type_identifiant_l'erreur> (à la Either en Haskell) donc en gros quelque chose qui se repose plus sur les types / le compilateur. En C++ on n'a pas que l'OO, faut pas l'oublier

[Davidbrcz]

Si je veux retourner un objet, j'utilise un paramètre passé par référence. D'ailleurs en règle générale c'est pas terrible qu'une fonction retourne directement un objet car ça fait une copie.

Oopa

Ben non pas forcément. Seules les fonctions susceptible de générer des erreurs auront leur résultat testé. Et ça, je ne vois pas comment l'éviter de toutes façon. A moins de faire un gros bloc try, mais c'est pas terrible.

Bah en théorie, toutes les fonctions sont susceptibles de générer des erreurs, que ca soit dans l'acquisition des ressources, dans le processus de traitement, ...

Comprends pô

En cas d'erreur, avec une exception, le flux des instructions est interrompu et on remonte de gestionnaire d'exception en gestionnaire jusqu'à soit en trouver un qui marche ou fermer le programme. Avec ta méthode, quand tu trouves une erreur tu fais quoi ? Tu continues sachant qu'il y a eu une erreur?

Après ça dépend de ce qu'on appelle une erreur.

Toutafé
Mais bon, ton exemple, tenter de supprimer un truc qui n'est pas présent, me laisse perplexe.

[JolyLoic]

Globalement d'accord avec les arguments échangés avant, sauf sur un point :

ou un truc à base de boost::variant<résultat_normal, type_identifiant_l'erreur> (à la Either en Haskell) donc en gros quelque chose qui se repose plus sur les types / le compilateur

Si on veut retourner un truc pouvant être en erreur, il me semble utile de signaler explicitement ce dont il s'agit, et ne pas utiliser une classe trop générique comme variant. boost::optional peut servir dans certains cas, et une classe nommée Faillible par exemple peut avoir un nom plus clair quand l'absence d'information est signe d'une erreur, et non simplement d'une absence d'information. Dans les cas où un code de retour est plus approprié qu'une exception, mais qui ne retourne pas de donnée en tant que tel, j'ai aussi une classe qui fait un assert dans son destructeur si sa valeur n'a pas été lue, ceci afin de forcer la lecture par l'appelant.

[Alp]

Si on veut retourner un truc pouvant être en erreur, il me semble utile de signaler explicitement ce dont il s'agit, et ne pas utiliser une classe trop générique comme variant. boost::optional peut servir dans certains cas, et une classe nommée Faillible par exemple peut avoir un nom plus clair quand l'absence d'information est signe d'une erreur, et non simplement d'une absence d'information.

Une classe utilitaire basée sur variant, dans la même idée qu'optional mais qui contiendrait une information *utile* au lieu de juste "j'ai échoué", c'est exactement ce dont je parle. Et là tu pourrais même lui donner plusieurs comportements : ignorer, lancer une exception, faire quelque chose avec information, ...
Il ne s'agit pas d'utiliser juste boost::variant brut comme ça, ce serait juste le mécanisme utilisé en interne
C'est exactement ce que fait le type paramétré Either de Haskell, et on peut chainer des opérations sur des Either via le fait que c'est une monade, donc avec l'opérateur >>=. J'ai commencé à mijoter une implémentation C++ de tout ça, c'est encore du W.I.P, pour voir si on peut rendre ça vraiment élégant et pratique.

[3DArchi]

Salut,
Bon, les inconvénients ont déjà été présentés.
Je me permets juste une remarque. GetLastError sous Win32 s'inscrit dans le cadre d'une API C qui date maintenant d'une bonne vingtaine d'année.

[Médinoc]

3DArchi m'ôte les mots de la bouche au dernier moment: Aussi bien BOOL + GetLastError() que le retour direct de code d'erreur (HRESULT ou autre) suivent une philosophie C et en C++, les exceptions sont préférables.

Mais cela ne signifie pas pour autant qu'il faille utiliser les exceptions en priorité: Je conseillerais de garder des fonctions avec code d'erreur dans les cas où l'erreur n'a rien exceptionnel (exemple: La différence que fait .Net entre Dictionary::Item[] (lance une exception) et Dictionary::TryGetValue() (retourne un booléen).
Ne serait-ce que pour éviter d'encombrer le debugger (sal***rie de XMLSerializer et ses FileNotFoundException)...

PS: Pour les histoires de multi-thread, je rappelle que GetLastError() (et la version Visual de errno) utilisent du TLS.

[white_tentacle]

Il y a aussi une autre bonne raison à utiliser GetLastError : quand les performances sont critiques.

La levée d'une exception est quelque chose de beaucoup plus lourd que le test d'une valeur de retour --> ça peut jouer dans certains cas.

[nikko34]

Il y a aussi une autre bonne raison à utiliser GetLastError : quand les performances sont critiques.

La levée d'une exception est quelque chose de beaucoup plus lourd que le test d'une valeur de retour --> ça peut jouer dans certains cas.

Certes mais généralement quand une exception est lâchée, les performances n'ont plus vraiment d'intérêt non? A moins de programmer par exception, ce qui n'est pas convenable.

[JolyLoic]

Il y a aussi une autre bonne raison à utiliser GetLastError : quand les performances sont critiques.

La levée d'une exception est quelque chose de beaucoup plus lourd que le test d'une valeur de retour --> ça peut jouer dans certains cas.

Si l'exception est traitée localement, oui. Si elle ne l'est pas, gérer l'équivalent de l'exception par codes de retour va conduire à faire plein de ifs, qui vont avoir un coût d'exécution, y compris dans les cas où l'exception n'est pas générée...

Je ne voir pas tant les exceptions comme un système de report d'erreur que comme un système de transport d'erreur.

[10_GOTO_10]

Si plusieurs threads utilisent la classe qui a le get last error là, un thread peut appeler GetLastError et récupérer l'erreur liée à un autre thread parce que l'autre thread entre temps a utilisé ton objet et le get last error a été modifié... ce genre de choses. Donc à moins que tu mette un gros lock sur tuot le code, ce qui va ralentir abominablement l'exécution et bloquer pas mal de threads...

Faux, au moins pour le GestLastError de Windows:

The last-error code is maintained on a per-thread basis. Multiple threads do not overwrite each other's last-error code.

( Source )

Je préfère encore une approche fortement orientée exceptions

Pas d'accord. Mais c'est un autre débat.

Le gros défaut du GetLastError, c'est qu'il faut l'appeler tout de suite après l'appel de la fonction, ce qui n'est pas toujours possible (plusieurs fonctions dans un if, par exemple). Et on oublie des fois que même la fonction GetLastError réinitialise le flag. Ce qui veut dire que si on fait:

Code :

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

1
2
3
4
if (GetLastError() != 0) {
  MaVariable = GetLastError();
  // Affichage MaVariable
  //...

Le premier GetLastError met à zéro le flag, et le second renvoie donc zéro dans MaVariable...

C'est comme ça qu'on voit des erreurs du genre "Erreur: L'opération s'est terminée avec succès"

[Alp]

Faux, au moins pour le GestLastError de Windows

Oui, j'ai vu après coup. Mais il est question de l'idiome ici. Donc, je vais plutôt formuler ça comme : si on implémente cet idiome, faire gaffe à ce que chaque thread ait "son GetLastError".

Pas d'accord. Mais c'est un autre débat.

Là, ça dépend fortement de la situations et de bien d'autres choses, pour une réponse objective. Il n'y a pas de règle générale qui dit que l'une est meilleure que l'autre.

[bruno_pages]

Je n'aime pas les exceptions, comme leur nom l'indique finalement elle ne devraient être utilisées qu'exceptionnellement . Comme il faut appeler les destructeurs sur les instances placées en pile lors de la descente liée à une levée d'exception le code produit pas le compilateur est beaucoup plus grand que sans (option de compilation).

Je suis d'accord avec les défauts indiqués à propos du getlasterror.

Mais plutôt que de rendre simplement un booleen disant ok/nok il faut utiliser une classe (disons Status) permettant de savoir à fois si c'est ok/nok et dans le cas nok qu'elle est l'erreur, celle-ci pouvant si besoin être enrichie/transformée par les appels imbriqués. Comme la chose est assez souvent recopiée (le retour d'une méthode devenant le retour de la méthode qui l'utilise + mémorisation locale etc) et qu'il faut que cela aille vite le contexte décrivant l'erreur et qui est lui aussi une instance (au niveau de Status c'est un pointeur vers une interface) à une gestion de type compteur faite dans Status pourqu'il soit libéré à la destruction du dernier Status qui l'a contenu. Bien évidemment il faut explicitement tester les retours de méthode retournant un Status pour savoir si il y a une erreur, et les 'vraies' valeurs retournées par les méthodes utilisent des paramètres de sortie, mais que voulez-vous, on a rien sans rien.

Donc quelque chose comme cela :

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
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
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
// interface
class Error {
  public:
    Error() : n_(0) {}
    virtual ~Error() {}
    virtual void print(ostream &) = 0; // par exemple
    void used() { n_ += 1; }
    void unused() { if (--n_ == 0) delete this; }
 
  private:
    int n_;
};
 
class Status {
  public:
    Status() : isok_(true), error_(0) {}
    Status(Error * e) : isok_(false), error_(e) { e->used(); }
    Status(const Status & st) {
      isok_ = st.isok_;
      if ((error_ = st.error_) != 0)
	error_->used();
    }
    Status & operator=(const Status & st) {
      if (&st != this) {
	if (error_ != 0) 
	  error_->unused();
	isok_ = st.isok_;
	if ((error_ = st.error_) != 0)
	  error_->used();
      }
      return *this;
    }	
    ~Status() { if (error_ != 0) error_->unused(); }
    bool isOk() const { return isok_; }
    const Error * error() const { return error_; }
    void print(ostream & o) { 
      if (error_ != 0)
	error_->print(o);
      else
	o << "ok" << endl;
    }
 
  private: 
    bool isok_;
    Error * error_;
};
 
// un cas d'erreur
class Error1 : public Error {
  public:
    Error1(string s) : s_(s) {}
    virtual void print(ostream & o) { o << s_ << endl; }
 
  private:
    string s_;
};
 
// un autre cas d'erreur
class Error2 : public Error {
  public:
    Error2(int v1, int v2) : v1_(v1), v2_(v2) {}
    virtual void print(ostream & o) {
      o << "value not between " << v1_ << " and " << v2_ << endl;
    }
 
  private:
    int v1_;
    int v2_;
};
 
// utilisation via des fonctions hadhoc pour exemple
 
Status f1(int v)
{
  if (v < 0)
    return Status(new Error1("the value is negative"));
 
  return Status(); // ok
}
 
Status f2(int v)
{
  Status r = f1(v);
 
  if (! r.isOk())
    return r;
 
  if ((v >= 2) && (v <= 10))
    return Status(); // ok
 
  return Status(new Error2(2, 10));
}

[edit]au fait, pas besoin de critiquer le fait que toutes les opérations soient inline, j'ai fais exprès pour limiter la hauteur du code [/edit]