L'utilisation de blocs try-catch imbriqués est-elle un anti-motif?

95

Est-ce un anti-modèle? C'est une pratique acceptable?

    try {
        //do something
    } catch (Exception e) { 
        try {
            //do something in the same line, but being less ambitious
        } catch (Exception ex) {
            try {
                //Do the minimum acceptable
            } catch (Exception e1) {
                //More try catches?
            }
        }
    }
Monsieur Smith
la source
Pouvez-vous nous donner des arguments pour cela? Pourquoi ne pouvez-vous pas gérer tous les types d'erreur dans la capture de premier niveau?
Morons
2
J'ai récemment vu ce type de code, exécuté par des programmeurs inexpérimentés qui ne savent pas vraiment ce qu'ils appellent à l'intérieur des blocs try et qui ne veulent pas tester le code. Dans l'exemple de code que j'ai vu, l'opération a été identique, mais effectuée à chaque fois avec des paramètres de secours.
Monsieur Smith,
@LokiAstari -Votre exemple est un essai dans la section Finally .. Où il n'y a pas de capture. Ceci est un imbriqué dans la section Try .. C'est différent.
Morons
4
Pourquoi devrait-il être un anti-modèle?
2
+1 pour "plus d'essayer de captures?"
JoelFan

Réponses:

85

Cela est parfois inévitable, surtout si votre code de récupération peut générer une exception.

Pas joli, mais parfois il n'y a pas d'alternative.

Oded
la source
17
@MisterSmith - Pas toujours.
Oded
4
Oui, c’est un peu ce que j’essayais de faire. Bien sûr, il arrive un moment dans vos déclarations imbriquées try / catch où il vous suffit de dire que cela suffit. Je plaidais en faveur de l'imbrication par opposition aux tentatives séquentielles try / catch, en disant qu'il y avait des situations dans lesquelles vous ne voulez que le code dans la seconde tentative à exécuter si le premier essai échoue.
AndrewC
5
@MisterSmith: Je préférerais des captures d'essais imbriquées à des captures d'essais séquentielles partiellement contrôlées par des variables de drapeau (si elles étaient fonctionnellement identiques).
FrustratedWithFormsDesigner
31
try {transaction.commit (); } catch {try {transaction.rollback (); } catch {seriouslogging ()} notsoseriouslogging (); } est un exemple de capture d'essai imbriquée nécessaire
Thanos Papathanasiou Le
3
Au moins, extrayez le bloc catch d'une méthode, les gars! Faisons au moins cela lisible.
M. Cochese
43

Je ne pense pas que ce soit un antipattern, mais largement utilisé à mauvais escient.

La plupart des captures d'essais imbriquées sont en effet évitables et laides en enfer, généralement le produit de développeurs débutants.

Mais il y a des moments où vous ne pouvez pas vous empêcher.

try{
     transaction.commit();
   }catch{
     logerror();
     try{
         transaction.rollback(); 
        }catch{
         seriousLogging();
        }
   }

En outre, vous aurez besoin d'un bool supplémentaire pour indiquer l'échec de la restauration ...

Thanos Papathanasiou
la source
19

La logique est bonne - dans certaines situations, il peut être parfaitement logique d'essayer une approche de repli, qui pourrait elle-même connaître des événements exceptionnels ... par conséquent, ce schéma est pratiquement inévitable.

Cependant, je suggérerais ce qui suit pour améliorer le code:

  • Refactorisez l’essai intérieur ... attrapez les blocs dans des fonctions séparées, par exemple attemptFallbackMethodet attemptMinimalRecovery.
  • Soyez plus précis sur les types d’exception particuliers qui sont interceptés. Vous attendez-vous vraiment à une sous-classe Exception et si oui, voulez-vous vraiment la gérer de la même manière?
  • Déterminez si un finallybloc pourrait avoir plus de sens - c'est généralement le cas pour tout ce qui ressemble à du "code de nettoyage de ressource"
Mikera
la source
14

Ça va. Une refactorisation à envisager consiste à intégrer le code dans sa propre méthode et à utiliser des sorties anticipées pour réussir, ce qui vous permet d'écrire les différentes tentatives de faire quelque chose au même niveau:

try {
    // do something
    return;
} catch (Exception e) {
    // fall through; you probably want to log this
}
try {
    // do something in the same line, but being less ambitious
    return;
} catch (Exception e) {
    // fall through again; you probably want to log this too
}
try {
    // Do the minimum acceptable
    return;
} catch (Exception e) {
    // if you don't have any more fallbacks, then throw an exception here
}
//More try catches?

Une fois que cela a été fait, vous pourriez envisager de le résumer dans un modèle de stratégie.

interface DoSomethingStrategy {
    public void doSomething() throws Exception;
}

class NormalStrategy implements DoSomethingStrategy {
    public void doSomething() throws Exception {
        // do something
    }
}

class FirstFallbackStrategy implements DoSomethingStrategy {
    public void doSomething() throws Exception {
        // do something in the same line, but being less ambitious
    }
}

class TrySeveralThingsStrategy implements DoSomethingStrategy {
    private DoSomethingStrategy[] strategies = {new NormalStrategy(), new FirstFallbackStrategy()};
    public void doSomething() throws Exception {
        for (DoSomethingStrategy strategy: strategies) {
            try {
                strategy.doSomething();
                return;
            }
            catch (Exception e) {
                // log and continue
            }
        }
        throw new Exception("all strategies failed");
    }
}

Ensuite, utilisez simplement le TrySeveralThingsStrategy, qui est une sorte de stratégie composite (deux modèles pour le prix d’un!).

Une mise en garde énorme: ne faites cela que si vos stratégies sont elles-mêmes suffisamment complexes ou si vous voulez pouvoir les utiliser de manière flexible. Sinon, vous ne disposez que de quelques lignes de code simple avec une énorme pile d'objets inutiles.

Tom Anderson
la source
7

Je ne pense pas que ce soit automatiquement un anti-modèle, mais je l'éviterais si je pouvais trouver un moyen plus facile et plus propre de faire la même chose. Si le langage de programmation dans lequel vous travaillez a une finallyconstruction, cela peut aider à résoudre ce problème, dans certains cas.

FrustratedWithFormsDesigner
la source
6

Pas un anti-motif en soi, mais un motif de code qui indique que vous devez refactoriser.

Et c'est assez facile, il vous suffit de connaître une règle générale qui consiste à n'écrire qu'un bloc try de la même manière. Si vous savez bien écrire le code associé, il suffit généralement de copier et coller chaque bloc try avec ses blocs catch et de le coller dans une nouvelle méthode, puis de remplacer le bloc d'origine par un appel à cette méthode.

Cette règle empirique est basée sur la suggestion de Robert C. Martin de son livre "Clean Code":

si le mot clé 'try' existe dans une fonction, il doit s'agir du tout premier mot de la fonction et il ne doit y avoir rien après les blocs catch / finally.

Un exemple rapide sur "pseudo-java". Supposons que nous ayons quelque chose comme ceci:

try {
    FileInputStream is = new FileInputStream(PATH_ONE);
    String configData = InputStreamUtils.readString(is);
    return configData;
} catch (FileNotFoundException e) {
    try {
        FileInputStream is = new FileInputStream(PATH_TWO);
        String configData = InputStreamUtils.readString(is);
        return configData;
    } catch (FileNotFoundException e) {
        try {
            FileInputStream is = new FileInputStream(PATH_THREE);
            String configData = InputStreamUtils.readString(is);
            return configData;
        } catch (FileNotFoundException e) {
            return null;
        }
    }
}

Ensuite, nous pourrions refactoriser chaque tentative d’attrape et dans ce cas, chaque bloc tentative-attrape essaiera la même chose mais à des endroits différents (quelle pratique: D), nous n’aurons qu’à copier-coller l’un des blocs tentative-attraper et à en faire une méthode .

public String loadConfigFile(String path) {
    try {
        FileInputStream is = new FileInputStream(path);
        String configData = InputStreamUtils.readString(is);
        return configData;
    } catch (FileNotFoundException e) {
        return null;
    }
}

Nous l'utilisons maintenant dans le même but qu'avant.

String[] paths = new String[] {PATH_ONE, PATH_TWO, PATH_THREE};

String configData;
for(String path : paths) {
    configData = loadConfigFile(path);
    if (configData != null) {
        break;
    }
}

J'espère que ça aide :)

Adrián Pérez
la source
bon exemple. Cet exemple est vraiment le genre de code que nous devons refactoriser. Cependant, d'autres fois, des captures imbriquées sont nécessaires.
linehrr
4

Cela diminue sûrement la lisibilité du code. Je dirais que si vous en avez la chance , évitez alors de faire des nids d'essayage.

Si vous devez imbriquer des prises d'essai, arrêtez-vous toujours pendant une minute et réfléchissez:

  • Ai-je la chance de les combiner?

    try {  
      ... code  
    } catch (FirstKindOfException e) {  
      ... do something  
    } catch (SecondKindOfException e) {  
      ... do something else    
    }
    
  • Devrais-je simplement extraire la partie imbriquée dans une nouvelle méthode? Le code sera beaucoup plus propre.

    ...  
    try {  
      ... code  
    } catch (FirstKindOfException e) {  
       panicMethod();  
    }   
    ...
    
    private void panicMethod(){   
    try{  
    ... do the nested things  
    catch (SecondKindOfException e) {  
      ... do something else    
      }  
    }
    

Il est évident que si vous devez imbriquer trois niveaux ou plus de captures d’essai, en une seule méthode, c’est un signe certain du temps pour le refactor.

CsBalazsHongrie
la source
3

J'ai vu ce modèle dans le code de réseau, et cela a du sens. Voici l'idée de base, en pseudocode:

try
   connect;
catch (ConnectionFailure)
   try
      sleep(500);
      connect;
   catch(ConnectionFailure)
      return CANT_CONNECT;
   end try;
end try;

Fondamentalement, c'est une heuristique. Une tentative de connexion infructueuse peut être simplement un problème réseau, mais si cela se produit deux fois, cela signifie probablement que la machine à laquelle vous essayez de vous connecter est vraiment inaccessible. Il existe probablement d'autres moyens de mettre en œuvre ce concept, mais ils seraient probablement encore plus laids que les tentatives imbriquées.

Maçon Wheeler
la source
2

J'ai résolu cette situation comme ceci (try-catch avec fallback):

$variableForWhichINeedFallback = null;
$fallbackOptions = array('Option1', 'Option2', 'Option3');
while (!$variableForWhichINeedFallback && $fallbackOptions){
    $fallbackOption = array_pop($fallbackOptions);
    try{
        $variableForWhichINeedFallback = doSomethingExceptionalWith($fallbackOption);
    }
    catch{
        continue;
    }
}
if (!$variableForWhichINeedFallback)
    raise new ExceptionalException();
Adrian
la source
2

J'ai "dû" faire cela dans une classe de test de manière coïncidente (JUnit), où la méthode setUp () devait créer des objets avec des paramètres de constructeur non valides dans un constructeur qui lançait une exception.

Si je devais faire échouer la construction de 3 objets invalides, par exemple, il me faudrait 3 blocs try-catch, imbriqués. J'ai créé une nouvelle méthode à la place, où les exceptions ont été interceptées et la valeur renvoyée était une nouvelle instance de la classe que je testais lorsqu'elle a réussi.

Bien sûr, je n'avais besoin que d'une méthode car j'ai fait la même chose 3 fois. Ce n'est peut-être pas une bonne solution pour les blocs imbriqués qui font des choses totalement différentes, mais au moins votre code deviendrait plus lisible dans la plupart des cas.

MarioDS
la source
0

Je pense en fait que c'est un anti-modèle.

Dans certains cas, vous voudrez peut-être avoir plusieurs tentatives, mais seulement si vous NE SAVEZ PAS quel type d'erreur vous cherchez, par exemple:

public class Test
{
    public static void Test()
    {            
        try
        {
           DoOp1();
        }
        catch(Exception ex)
        {
            // treat
        }

        try
        {
           DoOp2();
        }
        catch(Exception ex)
        {
            // treat
        }

        try
        {
           DoOp3();
        }
        catch(Exception ex)
        {
            // treat
        }
    }

    public static void Test()
    {
        try
        {
            DoOp1();
            DoOp2();
            DoOp3();
        }
        catch (DoOp1Exception ex1)
        {
        }
        catch (DoOp2Exception ex2)
        {
        }
        catch (DoOp3Exception ex3)
        {
        }
    }
}

Si vous ne savez pas ce que vous cherchez, vous devez utiliser la première manière, qui est à mon humble avis, moche et non fonctionnelle. Je suppose que ce dernier est beaucoup mieux.

Donc, si vous savez quel genre d'erreur vous recherchez, soyez précis . Pas besoin de tentatives de capture imbriquées ou multiples dans la même méthode.

George Silva
la source
2
Un code comme celui que vous avez montré n’a aucun sens dans la plupart sinon tous les cas. Cependant, le PO faisait référence à des captures imbriquées , ce qui est une question assez différente de celle qui s'applique à plusieurs déclarations consécutives.
JimmyB
0

Dans certains cas, un Try-Catch imbriqué est inévitable. Par exemple, lorsque le code de récupération d'erreur lui-même peut générer une exception. Mais pour améliorer la lisibilité du code, vous pouvez toujours extraire le bloc imbriqué dans une méthode qui lui est propre. Consultez ce billet de blog pour plus d'exemples sur les blocs Try-Catch-Finally imbriqués.

codelion
la source
0

Il n'y a rien mentionné comme Anti Pattern dans Java n'importe où. Oui, nous appelons peu de bonnes pratiques et de mauvaises pratiques.

Si un bloc try / catch est requis à l'intérieur d'un bloc catch, vous ne pouvez pas l'aider. Et il n'y a pas d'alternative. En tant que bloc catch ne peut pas fonctionner comme partie d’essai si une exception est levée.

Par exemple :

String str=null;
try{
   str = method(a);
}
catch(Exception)
{
try{
   str = doMethod(a);
}
catch(Exception ex)
{
  throw ex;
}

Dans l'exemple ci-dessus, la méthode lève une exception, mais la méthode doMethod (utilisée pour gérer l'exception de la méthode) lève même une exception. Dans ce cas, nous devons utiliser le catch try dans try try.

quelque chose qui est suggéré de ne pas faire est ..

try 
{
  .....1
}
catch(Exception ex)
{
}
try 
{
  .....2
}
catch(Exception ex)
{
}
try 
{
  .....3
}
catch(Exception ex)
{
}
try 
{
  .....3
}
catch(Exception ex)
{
}
try 
{
  .....4
}
catch(Exception ex)
{
}
Gauravprasad
la source
cela ne semble rien offrir de substantiel par rapport aux 12 réponses précédentes
gnat