OVH Cloud OVH Cloud

[TORDU] Exception dans un destructeur, oui mais...

32 réponses
Avatar
Aurélien REGAT-BARREL
Bonjour à tous,
tout d'abord bonne année.
Je souhaite améliorer la gestion des erreurs et le système de logs de mon
programme. J'ai ragardé un peu log4cpp, pour l'instant c'est trop usine à
gaz pour moi et ça me satisfait pas trop. Mais la syntaxe utilisée m'a
inspiré un nouveau style d'assertion. Le but est d'écrire ceci:

void DoSomething( int A ) // A doit être un chiffre
{
verify( A < 10 ) << "A n'est pas un chiffre : " << A;
// ...
}

Si l'assertion n'est pas vérifiée, alors le message qui suit sert à
construire un message d'erreur (loggé, affiché, ...) et une exception
VerificattionFailed est levée. Sinon tout continue.
Pour cela je me suis dit que verify() pouvait renvoyer une espèce de flux
qui dans son destructeur déclenche une exception (s'il le faut). Voici mon
programme de test qui fonctionne sous VC++ 7.1 et Devcpp (gcc version
3.3.1).

#include <iostream>
#include <sstream>
#include <string>

class VerificationFailed
{
public:
VerificationFailed( bool Throw ) : // si true lève l'exception dans le
destructeur
throw_( Throw ),
msg_( "VerificationFailed : " )
{}

~VerificationFailed()
{
if ( this->throw_ )
{
throw this->msg_;
}
}

template<typename T>
VerificationFailed & operator << ( const T & t )
{
if ( this->throw_ )
{
std::ostringstream oss;
oss << t;
this->msg_ += oss.str();
}
return *this;
}

private:
bool throw_;
std::string msg_;
};

VerificationFailed verify( bool b )
{
return VerificationFailed( !b );
}

int main()
{
try
{
int A = 9;
std::cout << "debut du programme\n";
verify( A < 10 ) << "A n'est pas un chiffre : " << A;
std::cout << "suite du programme\n";
++A;
verify( A < 10 ) << "A n'est pas un chiffre : " << A;
std::cout << "fin du programme\n";
}
catch ( const std::string & err )
{
std::cerr << err << '\n';
}
std::cin.ignore();
}

J'obtiens le résultat suivant:

debut du programme
suite du programme
VerificationFailed : A n'est pas un chiffre : 10

Cela semble donc marcher, bien que lever une exception dans un destructeur
soit une mauvaise pratique (c'est plutôt sur la règle de la durée de vie
d'un objet renvoyé par une fonction que je m'interroge). Serait-ce
l'exception qui confirme la règle ?
Merci à vous.

--
Aurélien REGAT-BARREL

10 réponses

1 2 3 4
Avatar
drkm
"Aurélien REGAT-BARREL" writes:

j'ai quasiment aucun destructeur (je me
suis converti au RAII).


Huh ?

--drkm

Avatar
James Kanze
drkm wrote:
"Aurélien REGAT-BARREL" writes:


Pour cela je me suis dit que verify() pouvait renvoyer une
espèce de flux qui dans son destructeur déclenche une
exception (s'il le faut).



A priori, je dirais que si l'un des opérateurs << lance une
exception, le destructeur du flux sera appelé, et là, BOOM !
ÀMHA, lancer une exception dans un destructeur est extrêmement
délicat.


Il faut pouvoir assurer qu'aucune exception ne sera lancée
entre la création et la destruction de chacune des instances.
Ce qui doit pourvoir néanmoins être fait dans certains cas
particuliers. Dans ce cas-ci, il faudrait imposer que les
surcharges de l'opérateur << pour ce flux ne lancent pas
d'exceptions.


Les règles des opérateurs dans la norme sont assez strictes. Si
la masque des exceptions dans ios interdit les exceptions,
aucune ne doit être lever, y compris dans le cas où le streambuf
en lève un. Si les operator<< sont écrit correctement, ils
atrappent les exceptions, et les mappe en appels à
ios::setstate.

Dans la pratique, fort peu d'opérateurs << (y compris les miens)
sont écrits correctement.

À tout hazard, dans ce cas-ci, je crois que j'emploierais mon
OutputStreamWrapper, modifié de façon à trapper et à ignorer
toute exception (ce qui a aussi l'avantage qu'aucune formattage
n'a lieu s'il n'y a pas de sortie.

Mais encore plus, je me poserais des questions sur l'utilisation
des exceptions dans ce cas-ci. Si j'ai bien compris, il s'agit
d'un genre de assertion, qu'il ne doit jamais y avoir de
l'exception si le programme est correct. Mais si le programme
n'est pas correct, commencer à déballer la pile en executant des
destructeurs sur des objets incohérents, c'est la dernière chose
que je veux faire.

--
James Kanze home: www.gabi-soft.fr
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 pl. Pierre Sémard, 78210 St.-Cyr-l'École, France +33 (0)1 30 23 00 34


Avatar
James Kanze
Aurélien REGAT-BARREL wrote:
Pourquoi lever une exception au lieu de tout arreter si c'est
une assertion ?



Ben l'exception est un bon moyen de tout arrêter proprement
non ?


Proprement, c'est rélatif. L'exception ne peut pas créer la
propreté une fois que c'est partie. En fait, il y a une risque
que les destructeurs l'empire. En fait, quand rien ne va comme
on s'y attendait, un core dump reste ce qu'il y a de plus
propre. Au moins on est sûr de ne pas faire plus de dégats qu'on
n'en a déjà fait.

Et évidemment, appeler abort() s'assure que dans le core dump,
on a tout le contexte pour pouvoir chercher l'erreur. Alors que
si l'exception nettoie la pile...

Je compte l'utiliser comme un assert() amélioré certes, mais
pas seulement, aussi voire surtout pour vérifier le code de
retour de fonctions systèmes / d'une bibliothèque tierce
(pilotage d'une caméra scientifique...). Si une telle fonction
échoue, je peux pas faire grand chose, mais pas la peine de
vautrer tout le logiciel. Cette exception sera catchée un peu
plus haut et transformée en OperationFailed, l'utilisateur
averti et puis c'est tout. Le log contiendra plus de détails
sur l'origine de l'échec (le message associé au verify()).


Ce n'est donc pas une véritable assertion.

Peut-être un paramètre supplémentaire à verify, pour signaler la
gravité de l'erreur et donc ce qu'il faut en faire -- avorter
le programme, le traitement en cours (exception) ou continuer
tout simplement.

- Si tu utilises ton 'verify' a l'intérieur d'un destructeur,
il faudra prendre garde a rattraper l'exception dans le
destructeur pour ne pas risquer d'avoir une destruction
inachevée.



Peu de risques que cela se produise, j'ai quasiment aucun
destructeur (je me suis converti au RAII).


? RAII, c'est de tout faire dans les destructeurs, plutôt que
dans une logique externe.

- Si tu inseres dans ton operator<< le résultat d'une fonction
qui leve une exception, tu auras une 2eme levée d'exception
non rattrapée lors de la destruction de la pile, ce qui est
justement le truc a éviter quand on dit de ne pas lever
d'exception dans un destructeur.



C'est effectivement une possibilité, genre:


verify( value != 0 ) << "La valeur de la classe " <<
this->getName() << " est nulle.";


si getName() lève une exception je suis mal...
Peu de risques malgré tout, bad_alloc me semble le principal.
Moyennant une bonne doc écrite en rouge clignotant, on devrait
pouvoir s'assurer de la chose.


Au niveau utilisation, vous en pensez quoi ? Je trouve ça bien
plus pratique que ce que j'ai fait jusque là:


if ( value >= 10 )
{
std::ostringstream oss;
oss << "La valeur n'est pas un chiffre : " << value;
throw std::logic_error( oss.str() );
}


Vous faîtes comment vous ?


C'est selon. Pour les asserts, j'ai :
Assert( <expr>, <message> ) ;
Ce n'est pas le plus parlant, mais vue que j'ai le core après.

Pour toute la reste, j'ai un loggeur qui renvoie un
OutputStreamWrapper, ce qui permet :

log( Log::fatal ) << "x == " << x << " <--- no go" ;

Si je le veux conditionnel, je peux :

log( x > 10 ? Log::fatal ? Log::info ) << ...

Note bien, en revanche, que fatal finit dans un appel à abort().
Je n'ai rien dans Log qui permettrait de lever une exception
(mais ça serait facile à ajouter -- OutputStreamWrapper utilisee
des callbacks pour l'action en fin des <<).

--
James Kanze home: www.gabi-soft.fr
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 pl. Pierre Sémard, 78210 St.-Cyr-l'École, France +33 (0)1 30 23 00 34


Avatar
James Kanze
Loïc Joly wrote:
Aurélien REGAT-BARREL wrote:


Pourquoi lever une exception au lieu de tout arreter si
c'est une assertion ?




Ben l'exception est un bon moyen de tout arrêter proprement
non ?



Ca se discute. Certains disent qu'en cas d'assert, tu n'es pas
certain de l'état dans lequel tu es, et donc il vaut mieux
arrêter tout au plus vite.


D'autres disent que même si le premier argument est vrai, si
le désagrément causé par un arrêt brutal est relativement
élevé, et si ce qui peut se passer au cas ou le programme
perdre totalement les pédales n'est pas dramatique, vu que la
probalité que ça se passe vraiment mal est assez faible
(estimation sans justification...), il vaut mieux lancer une
exception qui va arrêter calmement le programme.


J'avoue ne pas savoir me situer entre les deux camps.


Ça dépend un peu de l'application, peut-être. Tout ce que je
peux dire, c'est que dans mes applications, la règle à toujours
été de se barrer plutôt que de risquer plus de dégat. Mais il
faut dire qu'en général, j'ai eu un deuxième système en sécours,
qui réprenait la main, et que les dégats que pouvait faire le
programme étaient très élevés. (N'empèche que j'ai l'impression
que c'est une bonne règle de base. À condition de se rappeler
qu'il faut vérifier dans chaque application qu'on n'a pas
affaire à une exception.)

Vous faîtes comment vous ?



Je ne fais pas, mais je me demande si un truc tordu comme le
suivant pourrait marcher :


class DoOnceAndThrow
{
DoOnceAndThrow() isAlreadyDone(false) {}
void tryToDoIt()
{
if (isAlreadyDone)
throw std::exception;
isAlreadyDone = true;
}
bool isAlreadyDone;
}


#define verify(b)
DoOnceAndThrow doat##__LINE__; // 1
if (b) ; // 2
else
while(doOnceAndThrow())
log


// 1 : Pour essayer de ne pas avoir deux identificateurs
portant le même nom dans la même portée. Ne marchera que s'il
n'y a pas 2 verifie sur une seule ligne


// 2 : Permet d'éviter des trucs étranges si verifie est appelé dans un
if qui comporte un else, par exemple.


Et ça doit faire quoi, exactement ? Je ne comprends rien du code
dans le macro.

Et évidemment, ton if/else ne sert à rien ici. Parce que si
j'écris :

if ( a > b )
verify( c > b ) << ... ;

il n'y aurait que la declaration de doat##__LINE__ qui se trouve
dans l'if ; le if/else est une deuxième instruction. (Et si
j'ajoute un else à mon if, ci-dessus, ça ne compile même pas.

--
James Kanze home: www.gabi-soft.fr
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 pl. Pierre Sémard, 78210 St.-Cyr-l'École, France +33 (0)1 30 23 00 34



Avatar
Loïc Joly
James Kanze wrote:

Et ça doit faire quoi, exactement ? Je ne comprends rien du code
dans le macro.


Effectivement, j'ai eu une première version, je me suis rendu compte
d'un problème, j'ai voulu corriger, mais je l'ai fait uniquement à
moitié. Ce que je voulais écrire comme macro était :

class DoOnceAndThrow
{
DoOnceAndThrow() isAlreadyDone(false) {}
bool tryToDoIt()
{
if (isAlreadyDone)
throw std::exception;
isAlreadyDone = true;
return true;
}
bool isAlreadyDone;
}

#define verify(b)
DoOnceAndThrow doat##__LINE__;
if (b) ;
else
while(doat##__LINE__.tryToDoIt())
log



Et évidemment, ton if/else ne sert à rien ici. Parce que si
j'écris :

if ( a > b )
verify( c > b ) << ... ;

il n'y aurait que la declaration de doat##__LINE__ qui se trouve
dans l'if ; le if/else est une deuxième instruction. (Et si
j'ajoute un else à mon if, ci-dessus, ça ne compile même pas.


Tout à fait d'accord. Ce que c'est que d'avoir le nez dans le guidon.
Bon, nouvelle tentative (plus simple en fait) :

#define verify(b)
if (b) ;
else
for ( ; ; throw std::exception())
log


Ou encore plus simple :

#define verify(b)
for ( ; (b) ; throw std::exception())
log


Bon, c'est bien plus simple, j'espère qu'il y aura donc moins d'anneries...

--
Loïc

Avatar
Olivier Azeau
James Kanze wrote:
Aurélien REGAT-BARREL wrote:
Pourquoi lever une exception au lieu de tout arreter si c'est
une assertion ?


Ben l'exception est un bon moyen de tout arrêter proprement
non ?


Proprement, c'est rélatif. L'exception ne peut pas créer la
propreté une fois que c'est partie. En fait, il y a une risque
que les destructeurs l'empire. En fait, quand rien ne va comme
on s'y attendait, un core dump reste ce qu'il y a de plus
propre. Au moins on est sûr de ne pas faire plus de dégats qu'on
n'en a déjà fait.


C'est peut-être pour cela que j'ai l'impression que l'utilité des
exceptions est en fait très limitée :
- soit on est dans un cas où rien ne va plus (risque de corruption, ...)
et alors autant sortir immédiatement
- soit on est dans un cas où il y a erreur, mais cette erreur n'a rien
"d'exceptionnel", au sens où elle est prévue par le workflow et, là je
préfère avoir mon propre système pour gérer les cas de fonctionnement
"dégradés" : ils sont souvent gérés localement et le mécanisme de
remontée d'exception n'apporte rien.

En fin de compte, j'ai l'impression que la levée d'exception sert
surtout dans le cas d'un problème important qui impacte tout une
"grosse" sous-partie du programme, l'exception permettant alors de
détruire entièrement cette sous-partie et de redonner la main à un
niveau élevé qui va la reconstruire de zéro.

[snip]
Peu de risques que cela se produise, j'ai quasiment aucun
destructeur (je me suis converti au RAII).


? RAII, c'est de tout faire dans les destructeurs, plutôt que
dans une logique externe.


Je crois que Aurélien veut dire que le RAII répartit beaucoup plus la
logique dans divers destructeurs et qu'il n'a donc plus de destructeur
avec "beaucoup" de logique.



Avatar
Christophe Lephay
"Olivier Azeau" a écrit dans le message de news:
lbcGd.13468$
C'est peut-être pour cela que j'ai l'impression que l'utilité des
exceptions est en fait très limitée :
- soit on est dans un cas où rien ne va plus (risque de corruption, ...)
et alors autant sortir immédiatement
- soit on est dans un cas où il y a erreur, mais cette erreur n'a rien
"d'exceptionnel", au sens où elle est prévue par le workflow et, là je
préfère avoir mon propre système pour gérer les cas de fonctionnement
"dégradés" : ils sont souvent gérés localement et le mécanisme de remontée
d'exception n'apporte rien.


Là où le mécanisme des exception me semble approprié, c'est lorsqu'on n'a
pas la réponse sur le comment traiter l'erreur (ou la circonstance
"exceptionnelle") à l'endroit où elle est repérée.

Avatar
drkm
James Kanze writes:

À tout hazard, dans ce cas-ci, je crois que j'emploierais mon
OutputStreamWrapper, modifié de façon à trapper et à ignorer
toute exception (ce qui a aussi l'avantage qu'aucune formattage
n'a lieu s'il n'y a pas de sortie.


Tu veux dire en englobant l'appel à l'opérateur << du flux
encapsulé, dans le modèle de l'opérateur << de OutputStreamWrapper,
dans un bloc try ? Je n'y avais pas pensé. Ça me parait en effet une
solution.

Mais encore plus, je me poserais des questions sur l'utilisation
des exceptions dans ce cas-ci.


Moi aussi. Mais je ne suis pas certain qu'il s'agisse réellement
d'assertions. Je n'ai pas vérifié dans les articles précédents, mais
je peux imaginer un tel mécanisme pour vérifier des conditions qui ne
sont pas des assertions.

--drkm

Avatar
Olivier Azeau
Christophe Lephay wrote:
"Olivier Azeau" a écrit dans le message de news:
lbcGd.13468$

C'est peut-être pour cela que j'ai l'impression que l'utilité des
exceptions est en fait très limitée :
- soit on est dans un cas où rien ne va plus (risque de corruption, ...)
et alors autant sortir immédiatement
- soit on est dans un cas où il y a erreur, mais cette erreur n'a rien
"d'exceptionnel", au sens où elle est prévue par le workflow et, là je
préfère avoir mon propre système pour gérer les cas de fonctionnement
"dégradés" : ils sont souvent gérés localement et le mécanisme de remontée
d'exception n'apporte rien.



Là où le mécanisme des exception me semble approprié, c'est lorsqu'on n'a
pas la réponse sur le comment traiter l'erreur (ou la circonstance
"exceptionnelle") à l'endroit où elle est repérée.


Même dans un cas comme celui-là ?

class Erreur {};

class A
{
public:
int computeSomething( int x, int y ) {
if( y == 0 ) throw Erreur();
return x / y;
}

int computeSomeOtherThing( int x, int y ) {
if( x == 0 ) throw Erreur();
return y / x;
}
};

class B
{
public:
int computeAll( int x, int y )
{
A a;
int z1, z2;
try {
z1 = a.computeSomething( x, y );
try {
z2 = a.computeSomeOtherThing( z1-5, y );
}
catch( Erreur & ) {
z2 = 75;
}
}
catch( Erreur & ) {
z1 = 3;
try {
z2 = a.computeSomeOtherThing( x, y+5 );
}
catch( Erreur & ) {
z2 = 83;
}
}
return z1 + z2;
}
};

Je préfère dire que l'exception est appropriée quand elle simplifie
l'écriture du traitement de l'erreur...


Avatar
Aurelien REGAT-BARREL
Bon, désolé pour avoir parlé d'assertion. C'est verify() et pas assert(). Je
compte aussi m'en servir comme d'un assert(), mais avant tout comme d'un
moyen simple de faire des vérif et de lever des exceptions si elles ne sont
pas ok.
Par exemple, à la lecture d'un fichier de configuration, vous exigez que la
version de ce fichier soit "1".
Je trouve ceci :

int version = get_version();
verify( version == 1 ) << "numéro de version non supporté : " << version;

plus simple et parlant que :

int version = get_version();
if ( version != 1 )
{
std::ostringstream oss;
oss << "numéro de version non supporté : " << version;
Logger::Error( oss.str() );
throw OperationFailed( oss.str() );
}

Jusque là j'avais des fonctions genre:

void OperationFailedError( const char * msg, int n )
{
std::ostringstream oss;
oss << msg << n;
Logger::Error( oss.str() );
throw OperationFailed( oss.str() );
}

Et j'ai au moins 10 prototypes de OperationFailedError en fonction des
arguments voulus :

void OperationFailedError( const char * msg );
void OperationFailedError( const char * msg, int n );
void OperationFailedError( const char * msg1, int n, const char * msg2 );

D'ailleurs j'aurais pu faire ça avec des templates. Mais n'empêche que je
dois quand même écrire tout ça :

if ( version != 1 )
{
OperationFailedError( "numéro de version non supporté : ", version );
}

Arriver à condenser ce bloc en une seule ligne je trouve ça séduisant.

Car j'ai une question simple : comment faites vous pour gérer ce type de
vérifications, qui sont si fréquentes et barbantes à écrires ?

En fin de compte, j'ai l'impression que la levée d'exception sert surtout
dans le cas d'un problème important qui impacte tout une "grosse"
sous-partie du programme, l'exception permettant alors de détruire
entièrement cette sous-partie et de redonner la main à un niveau élevé qui
va la reconstruire de zéro.


Je veux m'en servir comme moyen fréquent de gestion des erreurs, c'est à
dire remplacer le code suivant :

// renvoie false si échec
bool ReadConfigFile( const std::string & FileName )
{
std::ifstream file( FileName.c_str() );
if ( !file )
{
Logger::Error( "Impossible de lire le fichier ", FileName );
return false;
}

int version;
if ( ! ( file >> version ) )
{
Logger::Error( "Impossible de lire la version du fichier",
FileName );
return false;
}
if ( version != 1 )
{
Logger::Error( "Numéro de version du fichier ", FileName, "
incorrect : ", version );
return false;
}
return true;
}

avec quelque chose comme ça :

// throw OperationFailed si échec
void ReadConfigFile( const std::string & FileName )
{
std::ifstream file( FileName.c_str() );

verify( file ) << "Impossible de lire le fichier " << FileName;

int version;
verify( file >> version ) << "Impossible de lire la version du fichier "
<< FileName;
verify( version == 1 ) << "Numéro de version du fichier "
<< FileName << " incorrect : ", version;
}

Par "je me suis converti au RAII", j'ai voulu signifier la disparition du
code de retour qui indique succès / échec, ce qui allège le code utilisateur
de ReadConfigFile :

if ( !ReadConfigFile( "toto.cfg" ) )
{
return false;
}
cout << "lecture okn";

Avec cette approche "RAII" appliquée aux fonctions en fait et non aux objets
:

ReadConfigFile( "toto.cfg" );
cout << "lecture okn";

si on se trouve à la ligne qui suit ReadConfigFile(), c'est que la lecture a
réussi. C'est ce que j'ai appelé RAII dans mon cas (par analogie au fait que
si la création d'un objet a réussi, alors c'est qu'il est utilisable). C'est
peut être pas le bon terme.
Je veux me servir des exceptions comme d'un flot d'exécution "parallèle"
réservé à la gestion des erreurs.

--
Aurélien REGAT-BARREL

1 2 3 4