Question

Je viens de vivre une expérience de dépannage assez pénible pour résoudre du code ressemblant à ceci:

try {
   doSomeStuff()
   doMore()
} finally {
   doSomeOtherStuff()
}

Le problème était difficile à résoudre car doSomeStuff () lançait une exception, ce qui a à son tour provoqué le déclenchement d'une exception par doSomeOtherStuff (). La deuxième exception (levée par le bloc finally) a été levée dans mon code, mais elle n'a pas de descripteur sur la première exception (levée à partir de doSomeStuff ()), qui était la véritable cause du problème.

Si le code l'avait plutôt indiqué, le problème aurait été évident:

try {
    doSomeStuff()
    doMore()
} catch (Exception e) {
    log.error(e);
} finally {
   doSomeOtherStuff()
}

Alors, ma question est la suivante:

Un bloc final utilisé sans aucun bloc d'arrêt est-il un anti-motif java bien connu? (Cela semble certainement être une sous-classe peu évidente de l'anti-pattern évidemment bien connu "Ne gobez pas les exceptions!")

Était-ce utile?

La solution

En général, non, ce n'est pas un anti-modèle. Enfin, l’objectif final est de s’assurer que tout est nettoyé, qu’une exception soit levée ou non. Le point essentiel de la gestion des exceptions est que, si vous ne pouvez pas vous en occuper, vous le laissez figer à quelqu'un qui le peut, grâce à la gestion des exceptions de signalisation hors bande relativement propre. Si vous devez vous assurer que les éléments sont nettoyés si une exception est générée, mais que vous ne pouvez pas gérer correctement l'exception dans l'étendue actuelle, c'est exactement la bonne chose à faire. Vous voudrez peut-être être un peu plus prudent pour vous assurer que votre bloc final ne sera pas lancé.

Autres conseils

Je pense que le vrai " anti-pattern " fait quelque chose dans un bloc finally qui peut lancer sans avoir un piège.

Pas du tout.

Ce qui ne va pas, c’est le code dans le finally.

N'oubliez pas que finalement, il sera toujours exécuté et qu'il est risqué (comme vous venez de le voir) de placer quelque chose qui peut générer une exception.

Il n'y a absolument rien de mal à essayer avec un attrape final et nul. Considérez ce qui suit:

InputStream in = null;
try {
    in = new FileInputStream("file.txt");
    // Do something that causes an IOException to be thrown
} finally {
    if (in != null) {
         try {
             in.close();
         } catch (IOException e) {
             // Nothing we can do.
         }
    }
}

Si une exception est levée et que ce code ne sait pas comment le gérer, l'exception doit faire remonter la pile d'appels vers l'appelant. Dans ce cas, nous souhaitons toujours nettoyer le flux, je pense donc qu'il est parfaitement logique de disposer d'un bloc try sans prise.

Je pense que c’est loin d’être un anti-modèle et c’est quelque chose que je fais très souvent quand il est essentiel de libérer les ressources obtenues lors de l’exécution de la méthode.

Une des choses que je fais avec les descripteurs de fichiers (pour l'écriture) consiste à vider le flux avant de le fermer à l'aide de la méthode IOUtils.closeQuietly, qui ne génère pas d'exceptions:


OutputStream os = null;
OutputStreamWriter wos = null;
try { 
   os = new FileOutputStream(...);
   wos = new OutputStreamWriter(os);
   // Lots of code

   wos.flush();
   os.flush();
finally {
   IOUtils.closeQuietly(wos);
   IOUtils.closeQuietly(os);
}

J'aime le faire de cette façon pour les raisons suivantes:

  • Il n'est pas totalement sûr d'ignorer une exception lors de la fermeture d'un fichier. S'il existe des octets qui n'ont pas encore été écrits dans le fichier, il est possible que le fichier ne soit pas dans l'état attendu par l'appelant;
  • Ainsi, si une exception est générée lors de la méthode flush (), elle sera propagée à l'appelant, mais je m'assurerai quand même que tous les fichiers sont fermés. La méthode IOUtils.closeQuietly (...) est moins détaillée que la tentative correspondante ... catch ... ignore me block;
  • Si vous utilisez plusieurs flux de sortie, l'ordre de la méthode flush () est important. Les flux créés en passant d'autres flux en tant que constructeurs doivent être vidés en premier. La même chose est valable pour la méthode close (), mais le flush () est plus clair à mon avis.

Je dirais qu'un bloc try sans bloc catch est un anti-motif. Dire "Ne pas avoir un enfin sans prise" est un sous-ensemble de "Ne pas essayer sans prise".

J'utilise try / finally sous la forme suivante:

try{
   Connection connection = ConnectionManager.openConnection();
   try{
       //work with the connection;
   }finally{
       if(connection != null){
          connection.close();           
       }
   }
}catch(ConnectionException connectionException){
   //handle connection exception;
}

Je préfère cela à l’essai / attrape / enfin (+ imbriqué essaie / attrape dans l’ultime) Je pense que c'est plus concis et que je ne duplique pas la capture (exception).

try {
    doSomeStuff()
    doMore()
} catch (Exception e) {
    log.error(e);
} finally {
   doSomeOtherStuff()
}

Ne faites pas ça non plus ... vous avez juste caché plus de bugs (enfin, vous ne les avez pas cachés exactement ... mais vous avez rendu plus difficile leur traitement. Quand vous attrapez Exception, vous attrapez aussi n'importe quelle RuntimeException (comme NullPointer et ArrayIndexOutOfBounds).

En général, attrapez les exceptions que vous devez attraper (exceptions vérifiées) et traitez les autres au moment du test. Les exceptions RuntimeExceptions sont conçues pour être utilisées pour les erreurs de programmation - et les erreurs de programmation sont des choses qui ne devraient pas se produire dans un programme correctement débogué.

À mon avis, il est plus fréquent que finalement avec un catch indique une sorte de problème. Le langage de la ressource est très simple:

acquire
try {
    use
} finally {
    release
}

En Java, vous pouvez avoir une exception à partir de n’importe où. Souvent, l’acquisition jette une exception vérifiée, la façon sensée de le gérer est de mettre un piège autour du comment. N'essayez pas de vérifier les nullités hideuses.

Si vous deviez être vraiment anal, notez qu'il existe des priorités implicites parmi les exceptions. Par exemple, ThreadDeath devrait tout englober, qu’il provienne de l’acquisition / utilisation / libération. Manier ces priorités correctement est inesthétique.

Abstenez-vous donc de la gestion de vos ressources avec l'idiome Execute Around.

Essayer / Enfin est un moyen de libérer correctement des ressources. Le code dans le bloc finally ne doit JAMAIS être lancé car il ne doit agir que sur les ressources ou l'état acquis AVANT l'entrée dans le bloc try.

En passant, je pense que log4J est presque un anti-modèle.

SI VOUS VOULEZ INSPECTER UN PROGRAMME EN COURS D'UTILISATION, UTILISEZ UN OUTIL D'INSPECTION APPROPRIÉ (c'est-à-dire un débogueur, un IDE ou, au sens le plus extrême, un code d'octet tisserand, mais NE METTEZ PAS DE DÉCLARATION DE CONSIGNES DANS TOUTES LES LIGNES!).

Dans les deux exemples que vous présentez, le premier semble correct. Le second inclut le code de l'enregistreur et introduit un bogue. Dans le deuxième exemple, vous supprimez une exception si les deux premières instructions en déclenchent une (c’est-à-dire que vous l’attrapez et vous la connectez, mais ne la revérifiez pas. C’est quelque chose que je trouve très courant dans l’utilisation de log4j et qui pose un réel problème de conception d’application. avec votre modification, vous faites échouer le programme d'une manière qui serait très difficile à gérer pour le système puisque vous avancez comme si vous n'aviez jamais eu d'exception (un peu comme VB basique sur l'erreur reprennent la construction suivante).

try-finally peut vous aider à réduire le code copier-coller dans le cas où une méthode comporte plusieurs instructions return . Prenons l'exemple suivant (Android Java):

boolean doSomethingIfTableNotEmpty(SQLiteDatabase db) {
    Cursor cursor = db.rawQuery("SELECT * FROM table", null);
    if (cursor != null) { 
        try {
            if (cursor.getCount() == 0) { 
                return false;
            }
        } finally {
            // this will get executed even if return was executed above
            cursor.close();
        }
    }
    // database had rows, so do something...
    return true;
}

S'il n'y avait pas de clause finally , vous devrez peut-être écrire cursor.close () deux fois: juste avant return false et aussi après la clause if environnante.

Je pense que l’essai sans prise est anti-pattern. Utiliser try / catch pour gérer des conditions exceptionnelles (erreurs d'E / S de fichier, délai d'attente de socket, etc.) n'est pas un anti-motif.

Si vous utilisez try / finally pour le nettoyage, utilisez plutôt un bloc using.

Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top