L'uso di blocchi try-catch nidificati è un anti-pattern?

74

Questo è un antipattern? È una pratica accettabile?

    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?
            }
        }
    }
    
posta Mister Smith 09.11.2011 - 17:16
fonte

14 risposte

69

Questo a volte è inevitabile, specialmente se il tuo codice di recupero potrebbe generare un'eccezione.

Non carina, ma a volte non ci sono alternative.

    
risposta data 09.11.2011 - 17:28
fonte
37

Non penso che sia un antipattern, solo ampiamente usato impropriamente.

La maggior parte dei tentativi di cattura annidati sono davvero evitabili e brutti da noi, di solito il prodotto di sviluppatori junior.

Ma ci sono momenti in cui non puoi evitarlo.

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

Inoltre, avrai bisogno di un ulteriore bool da qualche parte per indicare il rollback fallito ...

    
risposta data 09.11.2011 - 18:33
fonte
16

La logica va bene - può avere perfettamente senso in alcune situazioni per provare un approccio fallback, che potrebbe anch'esso sperimentare eventi eccezionali .... quindi questo schema è praticamente inevitabile.

Tuttavia suggerirei quanto segue per rendere il codice migliore:

  • Refactor the inner try ... cattura i blocchi in funzioni separate, ad es. attemptFallbackMethod e attemptMinimalRecovery .
  • Sii più specifico sui particolari tipi di eccezione che vengono catturati. Ti aspetti davvero una sottoclasse Exception any e in tal caso vuoi davvero gestirli tutti allo stesso modo?
  • Considera se un blocco finally potrebbe avere più senso: di solito è il caso di qualsiasi cosa che assomigli al "codice di pulitura delle risorse"
risposta data 09.11.2011 - 17:30
fonte
11

Va bene. Un refactoring da considerare è quello di spingere il codice nel proprio metodo e utilizzare le prime uscite per il successo, permettendoti di scrivere i diversi tentativi di fare qualcosa allo stesso livello:

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?

Una volta che l'hai scoppiata in questo modo, potresti pensare di avvolgerla in uno schema di strategia.

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

Quindi usa solo TrySeveralThingsStrategy , che è un tipo di strategia composita (due modelli per il prezzo di uno!).

Un enorme avvertimento: non farlo a meno che le tue strategie siano esse stesse sufficientemente complesse, o vuoi essere in grado di usarle in modi flessibili. Altrimenti, stai lardando qualche riga di codice semplice con un'enorme quantità di orientamento all'oggetto non necessario.

    
risposta data 09.04.2012 - 23:27
fonte
7

Non penso che sia automaticamente un anti-pattern, ma lo eviterei se trovassi un modo più semplice e più pulito di fare la stessa cosa. Se il linguaggio di programmazione su cui stai lavorando ha un costrutto finally , questo potrebbe aiutare a ripulirlo, in alcuni casi.

    
risposta data 09.11.2011 - 17:26
fonte
6

Non è un anti-pattern di per sé, ma un modello di codice che indica che è necessario refactoring.

Ed è abbastanza facile, devi solo sapere una regola empirica che non sta scrivendo più di un blocco try nello stesso metodo. Se conosci bene scrivere codice correlato insieme, di solito copi e incolla ogni blocco try con i suoi blocchi catch e lo incolli all'interno di un nuovo metodo, quindi sostituisci il blocco originale con una chiamata a questo metodo.

Questa regola empirica si basa sul suggerimento di Robert C. Martin dal suo libro "Codice pulito":

if the keyword 'try' exists in a function, it should be the very first word in the function and that there should be nothing after the catch/finally blocks.

Un rapido esempio su "pseudo-java". Supponiamo di avere qualcosa del genere:

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

Quindi potremmo refactoring ogni try try e in questo caso ogni blocco try-catch prova la stessa cosa ma in posizioni diverse (quanto è comodo: D), dobbiamo solo copiare uno dei blocchi try-catch e fare un metodo di esso.

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

Ora lo usiamo con lo stesso obiettivo di prima.

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

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

Spero che questo aiuti:)

    
risposta data 06.06.2015 - 01:51
fonte
4

Sta sicuramente diminuendo la leggibilità del codice. Direi se hai la possibilità , quindi evita i tentativi di annidamento.

Se devi annidare i tentativi di cattura, fermati sempre per un minuto e pensa:

  • ho la possibilità di combinarli?

    try {  
      ... code  
    } catch (FirstKindOfException e) {  
      ... do something  
    } catch (SecondKindOfException e) {  
      ... do something else    
    }
    
  • dovrei semplicemente estrarre la parte annidata in un nuovo metodo? Il codice sarà molto più pulito.

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

È ovvio che se devi nidificare tre o più livelli di try-catch, in un singolo metodo, questo è un sicuro segno di tempo per il refactoring.

    
risposta data 25.06.2014 - 17:00
fonte
3

Ho visto questo modello nel codice di rete, e in realtà ha senso. Ecco l'idea di base, in pseudocode:

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

Fondamentalmente è un euristico. Un tentativo fallito di connessione potrebbe essere solo un problema di rete, ma se succede due volte, probabilmente significa che la macchina alla quale stai cercando di connettersi è davvero irraggiungibile. Ci sono probabilmente altri modi per implementare questo concetto, ma molto probabilmente saranno anche più brutti dei tentativi nidificati.

    
risposta data 09.04.2012 - 23:45
fonte
2

Ho risolto questa situazione in questo modo (try-catch with 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();
    
risposta data 09.04.2012 - 16:19
fonte
2

Ho "dovuto" farlo in una classe di test per coincidenza (JUnit), in cui il metodo setUp () ha dovuto creare oggetti con parametri di costruttore non validi in un costruttore che ha generato un'eccezione.

Se dovessi fallire la costruzione di 3 oggetti non validi, ad esempio, avrei bisogno di 3 blocchi try-catch, annidati. Ho invece creato un nuovo metodo, in cui sono state rilevate le eccezioni e il valore restituito era una nuova istanza della classe che stavo testando quando è riuscita.

Naturalmente, avevo solo bisogno di 1 metodo perché ho fatto le stesse 3 volte. Potrebbe non essere una buona soluzione per i blocchi nidificati che fanno cose completamente diverse, ma almeno il tuo codice diventerebbe più leggibile nella maggior parte dei casi.

    
risposta data 09.04.2012 - 19:13
fonte
0

In realtà penso che sia un antipattern.

In alcuni casi potresti desiderare più try-catch, ma solo se NON CONOSCERTI che tipo di errore stai cercando, ad esempio:

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)
        {
        }
    }
}

Se non sai cosa stai cercando, DEVI usare il primo modo, che è IMHO, brutto e non funzionale. Immagino che quest'ultimo sia molto meglio.

Quindi, se sai quale TIPO di errore stai cercando, specifica . Non c'è bisogno di try-catch nidificati o multipli all'interno dello stesso metodo.

    
risposta data 09.11.2011 - 17:33
fonte
0

Sì, se li usi così. : -)

private ParseResult<Statement> parseStatement(int p) {
    try {
        return (ParseResult) parseIfElseStm(p);
    } catch (ParseException e1) {
    try {
        return (ParseResult) parseIfStm(p);
    } catch (ParseException e2) {
    try {
        return (ParseResult) parseWhileStm(p);
    } catch (ParseException e3) {
    try {
        return (ParseResult) parseIterationStm(p);
    } catch (ParseException e4) {
    try {
        return (ParseResult) parseForStm(p);
    } catch (ParseException e5) {
    try {
        return (ParseResult) parseReturnStm(p);
    } catch (ParseException e6) {
    try {
        return (ParseResult) parseLocalDefStm(p);
    } catch (ParseException e7) {
    try {
        return (ParseResult) parseExpStm(p);
    } catch (ParseException e8) {
    try {
        return (ParseResult) parseBlockStm(p);
    } catch (ParseException e9) {
    try {
        // Try the empty statement
        parseChar(p++, ';');
        return new ParseResult<Statement>(EmptyStm.INST, p);
    } catch (ParseException e10) {
        throw new ParseException("expecting statement");
    }}}}}}}}}}
}
    
risposta data 09.04.2012 - 19:43
fonte
0

In alcuni casi, una prova provvisoria annidata è inevitabile. Ad esempio quando il codice di ripristino di errore stesso può generare ed eccezione. Ma per migliorare la leggibilità del codice è sempre possibile estrarre il blocco nidificato in un metodo a sé stante. Controlla questo post sul blog per ulteriori esempi sui blocchi di Try-Catch-Finally nidificati.

    
risposta data 15.04.2015 - 02:04
fonte
-1

Non c'è nulla menzionato come Anti Pattern in java ovunque. Sì, chiamiamo poche cose buone pratiche e cattive pratiche.

Se è richiesto un blocco try / catch all'interno di un blocco catch, non è necessario farlo. E non c'è alternativa. Come blocco catch non può funzionare come try part se viene lanciata un'eccezione.

Per esempio:  String str = null; provare{    str = metodo (a); } catch (Exception) { provare{    str = doMethod (a); } cattura (eccezione ex) {   lanciare ex; }

Qui nel metodo di esempio sopra riportato si genera un'eccezione, ma il metodo doMethod (utilizzato per gestire l'eccezione del metodo) genera anche un'eccezione. In questo caso dobbiamo usare il try catch all'interno di try catch.

qualcosa che viene suggerito di non fare è ..

prova {   ..... 1 } cattura (eccezione ex) { } provare {   ..... 2 } cattura (eccezione ex) { } provare {   ..... 3 } cattura (eccezione ex) { } provare {   ..... 3 } cattura (eccezione ex) { } provare {   ..... 4 } cattura (eccezione ex) { }

    
risposta data 06.06.2015 - 01:09
fonte

Leggi altre domande sui tag