È 'catch (...) {throw; } 'una cattiva pratica?

74

Anche se sono d'accordo sul fatto che catturare ... senza ripensare è davvero sbagliato, credo comunque che l'uso di costrutti come questo:

try
{
  // Stuff
}
catch (...)
{
  // Some cleanup
  throw;
}

È accettabile nei casi in cui RAII non è applicabile . (Per favore, non chiedere ... non tutti nella mia compagnia piacciono la programmazione orientata agli oggetti e RAII è spesso visto come "materiale scolastico inutile" ...)

I miei colleghi dicono che dovresti sempre sapere quali eccezioni devono essere generate e che puoi sempre usare costrutti come:

try
{
  // Stuff
}
catch (exception_type1&)
{
  // Some cleanup
  throw;
}
catch (exception_type2&)
{
  // Some cleanup
  throw;
}
catch (exception_type3&)
{
  // Some cleanup
  throw;
}

C'è una buona pratica ben ammessa riguardo a queste situazioni?

    
posta ereOn 05.12.2011 - 10:05
fonte

6 risposte

189

My coworkers says that you should always know what exceptions are to be thrown [...]

Il tuo collega, mi dispiacerebbe dirlo, non ha ovviamente mai lavorato su librerie generiche.

In che modo una classe come std::vector persino fa finta per sapere cosa lanciano i costruttori di copie, garantendo comunque la sicurezza delle eccezioni?

Se sapessi sempre che cosa potrebbe fare il callee in fase di compilazione, il polimorfismo sarebbe inutile! A volte l'intero obiettivo è quello di astrarre ciò che accade a un livello inferiore, quindi in particolare non vuoi sapere cosa sta succedendo!

    
risposta data 05.12.2011 - 10:07
fonte
43

Ciò di cui sembri catturato è l'inferno specifico di qualcuno che cerca di avere la torta e di mangiarla anch'essa.

RAII ed eccezioni sono progettati per andare di pari passo. RAII è il mezzo con cui non hai per scrivere un sacco di dichiarazioni catch(...) per fare pulizia. Accadrà automaticamente, naturalmente. E le eccezioni sono l'unico modo per lavorare con gli oggetti RAII, perché i costruttori possono solo riuscire o lanciare (o mettere l'oggetto in uno stato di errore, ma chi lo vuole?).

Un'istruzione catch può fare una di queste due cose: gestire un errore o una circostanza eccezionale, o eseguire lavori di pulizia. A volte lo fa entrambi, ma ogni dichiarazione catch esiste per fare almeno uno di questi.

catch(...) non è in grado di eseguire correttamente la gestione delle eccezioni. Non sai cosa sia l'eccezione; non puoi ottenere informazioni sull'eccezione. Non hai assolutamente altre informazioni oltre al fatto che un'eccezione è stata lanciata da qualcosa all'interno di un certo blocco di codice. L'unica cosa legittima che puoi fare in un blocco del genere è fare pulizia. E questo significa ri-lanciare l'eccezione alla fine della pulizia.

Ciò che RAII ti offre in merito alla gestione delle eccezioni è la pulizia gratuita. Se tutto RAII è incapsulato correttamente, allora tutto sarà pulito correttamente. Non è più necessario avere istruzioni di catch di pulizia. In tal caso, non c'è motivo di scrivere un'istruzione catch(...) .

Quindi concordo sul fatto che catch(...) sia perlopiù malvagio ... provvisoriamente .

Tale disposizione è un uso corretto di RAII. Perché senza di esso, hai bisogno di essere in grado di fare una certa pulizia. Non c'è modo di aggirarlo; devi essere in grado di fare lavori di pulizia. Devi essere in grado di assicurarti che il lancio di un'eccezione lascerà il codice in uno stato ragionevole. E catch(...) è uno strumento vitale nel farlo.

Non puoi averne uno senza l'altro. Non puoi dire che sia RAII che catch(...) sono cattivi. Hai bisogno di almeno uno di questi; altrimenti, non sei eccezionalmente sicuro.

Ovviamente, c'è un uso valido, anche se raro, di catch(...) che nemmeno RAII può bandire: ottenere un exception_ptr da inoltrare a qualcun altro. In genere attraverso un promise/future o un'interfaccia simile.

My coworkers says that you should always know what exceptions are to be thrown and that you can always use constructs like:

Il tuo collega è un idiota (o semplicemente terribilmente ignorante). Questo dovrebbe essere immediatamente ovvio a causa della quantità di codice copia e incolla che sta suggerendo di scrivere. la pulizia di ciascuna di queste istruzioni catch sarà esattamente la stessa . Questo è un incubo di manutenzione, per non parlare della leggibilità.

In breve: questo è il problema che RAII è stato creato per risolvere (non che non risolva altri problemi).

Ciò che mi confonde su questa nozione è che è generalmente retrocesso a come la maggior parte delle persone sostiene che RAII è cattivo. Generalmente, l'argomento dice "RAII è male perché devi usare le eccezioni per segnalare il fallimento del costruttore, ma non puoi lanciare eccezioni, perché non è sicuro e devi avere un sacco di dichiarazioni di catch per pulire tutto. " Che è un argomento rotto perché RAII risolve il problema che la mancanza di RAII crea.

Più che probabile, è contro RAII perché nasconde i dettagli. Le chiamate al distruttore non sono immediatamente visibili sulle variabili automatiche. Quindi ottieni il codice che viene chiamato implicitamente. Alcuni programmatori lo odiano davvero. Apparentemente, al punto in cui pensano di avere il 3% di dichiarazioni dicatch, tutte cose che fanno la stessa cosa con il codice copia-incolla è un'idea migliore.

    
risposta data 05.12.2011 - 12:00
fonte
14

Due commenti, davvero. Il primo è quello mentre sei in un mondo ideale, tu dovrebbe sempre sapere quali eccezioni potrebbero essere lanciate, in pratica, se hai a che fare con librerie di terze parti, o compilare con una Microsoft compilatore, non lo fai. Più precisamente al punto, tuttavia; anche se lo sai esattamente tutte le possibili eccezioni, è rilevante qui? catch (...) esprime l'intento molto meglio di catch ( std::exception const& ) , anche supponendo che tutte le possibili eccezioni derivino da std::exception (che sarebbe il caso in un mondo ideale). Quanto a usando diversi blocchi di cattura, se non esiste una base comune per tutti eccezioni: questa è completamente offuscata e un incubo di manutenzione. Come riconosci che tutti i comportamenti sono identici? E quello quello era l'intento? E cosa succede se devi cambiare il comportamento (bug fix, ad esempio)? È fin troppo facile non vederne uno.

    
risposta data 05.12.2011 - 10:21
fonte
11

Penso che il tuo collega abbia mescolato qualche buon consiglio: dovresti gestire solo le eccezioni conosciute in un blocco catch quando non le rileggi.

Questo significa:

try
{
  // Stuff
}
catch (...)
{
  // General stuff
}

È cattivo perché nasconde silenziosamente qualsiasi errore.

Tuttavia:

try
{
  // Stuff
}
catch (exception_type_we_can_handle&)
{
  // Deal with the known exception
}

Va bene: sappiamo cosa abbiamo a che fare e non è necessario esporlo al codice chiamante.

Allo stesso modo:

try
{
  // Stuff
}
catch (...)
{
  // Rollback transactions, log errors, etc
  throw;
}

Va bene, anche le migliori pratiche, il codice per gestire gli errori generali dovrebbe essere con il codice che li causa. È meglio che affidarsi al chiamato per sapere che una transazione ha bisogno di essere ripristinata o quant'altro.

    
risposta data 05.12.2011 - 11:27
fonte
9

Qualsiasi risposta di o no dovrebbe essere accompagnata da una motivazione del perché sia così.

Dire che è sbagliato semplicemente perché mi è stato insegnato in quel modo è solo fanatismo cieco.

Scrivendo lo stesso //Some cleanup; throw più volte, come nel tuo esempio è sbagliato perché è la duplicazione del codice e questo è un onere di manutenzione. Scriverlo solo una volta è meglio.

Scrivere un catch(...) per silenziare tutte le eccezioni è sbagliato, perché dovresti gestire solo le eccezioni che sai come gestire, e con quel carattere jolly puoi cath più di quanto ti aspetti, e così facendo puoi mettere a tacere errori importanti.

Ma se retrocedi dopo un catch(...) , allora la seconda logica non si applica più, poiché in realtà non stai gestendo l'eccezione, quindi non c'è motivo per cui questo dovrebbe essere scoraggiato.

In realtà l'ho fatto per accedere a funzioni sensibili senza alcun problema:

void DoSomethingImportant()
{
    try
    {
        Log("Going to do something important");
        DoIt();
    }
    catch (std::exception &e)
    {
        Log("Error doing something important: %s", e.what());
        throw;
    }
    catch (...)
    {
        Log("Unexpected error doing something important");
        throw;
    }
    Log("Success doing something important");
}
    
risposta data 05.12.2011 - 10:32
fonte
2

In generale sono d'accordo con lo stato d'animo dei post qui, non mi piace il modello di cattura di eccezioni specifiche - penso che la sintassi di questo è ancora nella sua infanzia e non è ancora in grado di gestire il codice ridondante.

Ma dal momento che tutti lo stanno dicendo, mi dilungherò sul fatto che, anche se li uso con parsimonia, ho spesso guardato una delle mie dichiarazioni "catch (Exception e)" e ho detto "Accidenti, vorrei che Ho chiamato le eccezioni specifiche di quel momento "perché quando entri più tardi è spesso bello sapere qual è l'intenzione e ciò che il cliente è in grado di lanciare a colpo d'occhio.

Non giustifico l'atteggiamento di "Usa sempre x", solo dicendo che occasionalmente è bello vederli elencati e sono sicuro che questo è il motivo per cui alcune persone pensano che sia la strada "giusta" per andare.

    
risposta data 05.12.2011 - 19:05
fonte

Leggi altre domande sui tag