2009-03-02 1 views
43

Ho appena avuto un'esperienza di risoluzione dei problemi abbastanza doloroso per la risoluzione di un codice che si presentava così:È un blocco finale senza blocco catch un anti-pattern java?

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

Il problema era difficile da risolvere perché doSomeStuff() ha generato un'eccezione, che a sua volta ha causato doSomeOtherStuff() per lanciare anche un'eccezione. La seconda eccezione (generata dal blocco finally) è stata generata sul mio codice, ma non aveva un handle sulla prima eccezione (generata da doSomeStuff()), che era la vera causa del problema.

Se il codice avesse detto questo, invece, il problema sarebbe stato subito evidente:

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

Quindi, la mia domanda è questa:

è un blocco finally usato senza alcun blocco catch una ben noto anti-pattern java? (Certamente sembra essere una sottoclasse non facilmente evidente del ben noto anti-pattern "Non trangugiare le eccezioni!")

+0

Vedere anche una domanda simile su [try-finally senza catch in C#] (http://stackoverflow.com/questions/128818/why-is-try-finally-good-try-catch-bad) - the stessi argomenti si applicano. – avandeursen

risposta

61

In generale, no, questo non è un anti-pattern. Il punto dei blocchi finali è quello di assicurarsi che le cose vengano ripulite se viene lanciata un'eccezione o meno. L'intero punto di gestione delle eccezioni è che, se non si è in grado di gestirlo, si lascia che diventi un problema a qualcuno che, attraverso la gestione delle eccezioni di segnalazione out-of-band relativamente pulita, fornisce. Se è necessario assicurarsi che le cose vengano ripulite se viene lanciata un'eccezione, ma non possono gestire correttamente l'eccezione nell'ambito corrente, allora questa è esattamente la cosa giusta da fare. Potresti voler fare un po 'più attenzione per assicurarti che il tuo blocco finale non venga lanciato.

+0

Ma in questo caso, l'eccezione di causa principale non era consentita a esplodere, perché l'ha finalmente calpestato (causandone così l'inghippo). – Jared

+0

Ok, ma questo è un caso d'angolo. Stai chiedendo se try-finally w/o catch è un anti-pattern in generale. – dsimcha

+1

concordato, dsimcha. Spero che ci sia consenso sul fatto che, in generale, questo non sia un anti-modello. Forse un anti-modello è, "Infine blocca le eccezioni di lancio"? – Jared

0

Direi che un blocco try senza blocco catch è un anti- modello. Dire "Non avere un colpo senza presa" è un sottoinsieme di "Non provare senza cattura".

+0

Sono d'accordo, non vedo perché questo sarebbe downvoted. – BobbyShaftoe

+2

non vale un voto negativo, ma non è un anti-modello. un blocco finally consente a un determinato pezzo di codice di essere sempre eseguito, come la chiusura delle risorse, o il rilascio dei blocchi, che potrebbero non generare eccezioni. – Chii

+0

Il mio punto era che il Catch mancante era l'anti-pattern di cui sopra, a prescindere dal fatto che una Finale fosse presente o meno. Eccezionare le eccezioni è ciò che ha causato il problema del debug, esacerbato dal codice eccezionalmente possibile in Infine. – billjamesdev

10

Non c'è assolutamente niente di sbagliato in una prova con una cattura definitiva e senza cattura. Si consideri il seguente:

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

Se viene generata un'eccezione e questo codice non sa come affrontare poi l'eccezione dovrebbe bollire lo stack di chiamate al chiamante. In questo caso vogliamo ancora ripulire il flusso, quindi penso che sia perfettamente logico avere un blocco try senza cattura.

+0

Se si rimuove il catch (nidificato infine) da questo esempio, se il try genera IOException e anche il close() lo fa, non si può sapere dallo stack trace che la causa principale del problema è l'IOException originale. E 'solo uno di questi casi, "Sì, sarà sempre difficile"? – Jared

+1

@ Jared: Sì, sarà sempre difficile. :) –

+0

FYI - quando hai rimosso il blocco dal blocco finally, hai praticamente detto che non ti importava da dove proveniva l'eccezione ... –

26

Penso che il vero "anti-pattern" qui stia facendo qualcosa in un blocco finally che può lanciare, non non avere un problema.

1

Io uso try/finally nella forma seguente:

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

preferisco questo al try/catch/finally (+ annidati try/catch nel finalmente). Penso che sia più conciso e non duplo il fermo (eccezione).

+0

Non soffre dello stesso problema di un'eccezione nel divorare finalmente un'eccezione nel tentativo? – Jared

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

Non fare che o ... appena nascosto più bug (anche non esattamente li aveva nascosti ... ma ha reso più difficile trattare con loro. Quando si cattura l'eccezione si sta anche recuperando ogni sorta di RuntimeException (come NullPointer e ArrayIndexOutOfBounds)

In generale, rileva le eccezioni che devi catturare (eccezioni controllate) e gestisci le altre in fase di test.Le eccezioni di Runtime sono progettate per essere utilizzate per errori del programmatore - e gli errori del programmatore sono cose ciò non dovrebbe accadere in un programma debitamente corretto.

+0

Sì ... d'accordo. Questo continua a indicare che il vero anti-modello è un qualcosa che alla fine lancia un'eccezione. – Jared

-1

Penso che provare senza cattura sia anti-pattern. L'utilizzo di try/catch per gestire le condizioni eccezionali di (errori di I/O del file, timeout del socket, ecc.) Non è un anti-pattern.

Se si sta utilizzando try/finally per la pulizia, prendere in considerazione un blocco utilizzando invece.

16

Niente affatto.

Ciò che è sbagliato è il codice all'interno infine.

Ricorda che alla fine verrà sempre eseguito, ed è solo rischioso (come hai appena visto) di mettere qualcosa che può generare un'eccezione lì.

5

Penso che sia lontano dall'essere un anti-pattern ed è qualcosa che faccio molto frequentemente quando è fondamentale rilasciare risorse ottenute durante l'esecuzione del metodo.

Una cosa che faccio quando si tratta di handle di file (per la scrittura) è il lavaggio del torrente prima di chiuderlo con il metodo IOUtils.closeQuietly, che non genera eccezioni:

 

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

Mi piace fare in modo che modo per i seguenti motivi:

  • non è completamente sicuro di ignorare un'eccezione quando si chiude un file - se ci sono i byte che non sono stati scritti nel file ancora, allora il file non può essere nello stato del chiamante sarebbe aspettarsi;
  • Quindi, se viene sollevata un'eccezione durante il metodo flush(), verrà propagata al chiamante, ma continuo a verificare che tutti i file siano chiusi. Il metodo IOUtils.closeQuietly (...) è meno dettagliato del corrispondente try ... catch ... ignora il blocco di me;
  • Se si utilizzano più flussi di output, l'ordine per il metodo flush() è importante. I flussi che sono stati creati passando altri stream come costruttori dovrebbero essere svuotati per primi. La stessa cosa è valida per il metodo close(), ma il flush() è più chiaro a mio avviso.
1

A mio parere, è più che finally con un catch indicare un qualche tipo di problema. L'idioma della risorsa è molto semplice:

acquire 
try { 
    use 
} finally { 
    release 
} 

In Java è possibile avere un'eccezione praticamente ovunque. Spesso l'acquisizione genera un'eccezione verificata, il modo ragionevole di gestirlo è quello di mettere un freno intorno al lotto. Non provare un orribile controllo nullo.

Se stavi per essere davvero anale, dovresti notare che ci sono delle priorità implicite tra le eccezioni. Ad esempio ThreadDeath dovrebbe clobberare tutto, sia che provenga da acquisire/utilizzare/rilasciare. Gestire correttamente queste priorità è sgradevole.

Pertanto, astrarre le risorse che gestiscono con l'idioma di Execute Around.

0

Try/Finally è un modo per liberare correttamente le risorse. Il codice nel blocco finally non dovrebbe MAI lanciare poiché dovrebbe agire solo sulle risorse o sullo stato che è stato acquisito PRIMA di entrare nel blocco try.

Per inciso, penso che log4J sia quasi un anti-pattern.

SE VUOI ISPEZIONARE UN PROGRAMMA IN CORSO, UTILIZZARE UN STRUMENTO DI CONTROLLO IDEALE (ad esempio un debugger, IDE, o in un senso estremo un codificatore di byte code ma NON METTERE LE DICHIARAZIONI DI LOGGING IN OGNI PUNTA!).

Nei due esempi che presenti il ​​primo sembra corretto. Il secondo include il codice logger e introduce un bug. Nel secondo esempio si sopprime un'eccezione se una viene lanciata dalle prime due istruzioni (cioè la si cattura e la si registra ma non si ripresenta. Questo è qualcosa che trovo molto comune nell'uso di log4j ed è un vero problema di progettazione dell'applicazione. con il tuo cambiamento fai fallire il programma in un modo che sarebbe molto difficile da gestire dato che praticamente continui come se non avessi mai un'eccezione (sorta come VB basic su errore riprendi il costrutto successivo)

0

try-finally può aiutare a ridurre il codice copia-incolla nel caso in cui un metodo ha più return dichiarazioni si consideri il seguente esempio (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; 
} 

Se ci fosse no finally clausola, potrebbe essere necessario scrivere cursor.close() due volte: appena prima return false e anche dopo la circostanza if circostante.