Perché i blocchi di cattura vuoti sono una cattiva idea? [chiuso]

StackOverflow https://stackoverflow.com/questions/1234343

  •  22-07-2019
  •  | 
  •  

Domanda

Ho appena visto una su try-catch , quali persone (incluso Jon Skeet) dicono che i blocchi di cattura vuoti sono davvero una cattiva idea? Perchè questo? Non esiste una situazione in cui una presa vuota non è una decisione di progettazione sbagliata?

Voglio dire, ad esempio, a volte vuoi ottenere alcune informazioni aggiuntive da qualche parte (servizio web, database) e davvero non ti importa se otterrai queste informazioni o meno. Quindi provi a ottenerlo, e se succede qualcosa, va bene, aggiungerò solo un " catch (eccezione ignorata) {} " e questo è tutto

È stato utile?

Soluzione

Di solito, try-catch vuoto è una cattiva idea perché stai ingoiando silenziosamente una condizione di errore e poi continuando l'esecuzione. Occasionalmente questa potrebbe essere la cosa giusta da fare, ma spesso è un segno che uno sviluppatore ha visto un'eccezione, non sapeva cosa fare al riguardo e quindi ha usato una cattura vuota per mettere a tacere il problema.

È l'equivalente di programmazione del mettere del nastro nero sopra una spia del motore.

Credo che il modo in cui gestisci le eccezioni dipende dal livello del software in cui stai lavorando: Eccezioni nella foresta pluviale .

Altri suggerimenti

Sono una pessima idea in generale perché è una condizione davvero rara in cui un fallimento (condizione eccezionale, più genericamente) viene correttamente incontrato senza alcuna risposta. Inoltre, i blocchi catch vuoti sono uno strumento comune utilizzato dalle persone che utilizzano il motore delle eccezioni per verificare se devono fare preventivamente.

Dire che è sempre cattivo non è vero ... questo è vero per ben poco. Ci possono essere circostanze in cui o non ti interessa che si sia verificato un errore o che la presenza dell'errore indichi in qualche modo che non puoi fare nulla al riguardo (ad esempio, quando scrivi un errore precedente in un file di registro di testo e ottieni un IOException , il che significa che non puoi comunque scrivere il nuovo errore).

Non allungherei le cose fino a dire che chi usa blocchi di cattura vuoti è un cattivo programmatore e non sa cosa sta facendo ...

Uso blocchi di cattura vuoti se necessario. A volte il programmatore della biblioteca che sto consumando non sa cosa sta facendo e genera eccezioni anche in situazioni in cui nessuno ne ha bisogno.

Ad esempio, considera alcune librerie del server http, non mi può fregare di meno se il server genera un'eccezione perché il client si è disconnesso e index.html non è stato inviato.

Ci sono rari casi in cui può essere giustificato. In Python vedi spesso questo tipo di costruzione: 

try:
    result = foo()
except ValueError:
    result = None

Quindi potrebbe essere OK (a seconda dell'applicazione) fare: 

result = bar()
if result == None:
    try:
        result = foo()
    except ValueError:
        pass # Python pass is equivalent to { } in curly-brace languages
 # Now result == None if bar() returned None *and* foo() failed

In un recente progetto .NET, ho dovuto scrivere codice per enumerare le DLL dei plug-in per trovare le classi che implementano una determinata interfaccia. Il bit di codice rilevante (in VB.NET, scusate) è: 

    For Each dllFile As String In dllFiles
        Try
            ' Try to load the DLL as a .NET Assembly
            Dim dll As Assembly = Assembly.LoadFile(dllFile)
            ' Loop through the classes in the DLL
            For Each cls As Type In dll.GetExportedTypes()
                ' Does this class implement the interface?
                If interfaceType.IsAssignableFrom(cls) Then

                    ' ... more code here ...

                End If
            Next
        Catch ex As Exception
            ' Unable to load the Assembly or enumerate types -- just ignore
        End Try
    Next

Anche se anche in questo caso, ammetto che la registrazione dell'errore da qualche parte sarebbe probabilmente un miglioramento.

I blocchi di cattura vuoti vengono generalmente inseriti perché il programmatore non sa davvero cosa stanno facendo. Nella mia organizzazione, un blocco di cattura vuoto deve includere un commento sul perché non fare nulla con l'eccezione è una buona idea.

In una nota correlata, la maggior parte delle persone non sa che un blocco try {} può essere seguito con un catch {} o infine {}, ne è richiesto solo uno.

Le eccezioni dovrebbero essere lanciate solo se esiste davvero un'eccezione - qualcosa che accade oltre la norma. Un blocco di cattura vuoto in pratica dice "sta succedendo qualcosa di brutto, ma non mi interessa". Questa è una cattiva idea.

Se non si desidera gestire l'eccezione, lasciarla propagare verso l'alto fino a raggiungere un codice in grado di gestirla. Se nulla è in grado di gestire l'eccezione, l'applicazione dovrebbe essere disattivata.

Penso che vada bene se prendi un tipo di eccezione particolare di cui sai che verrà sollevato solo per un motivo particolare e ti aspetti che l'eccezione e davvero non c'è bisogno di fare nulla al riguardo.

Ma anche in quel caso, un messaggio di debug potrebbe essere in ordine.

Per Josh Bloch - Articolo 65: non ignorare le eccezioni di Java efficace :

  1. Un blocco catch vuoto vanifica lo scopo delle eccezioni
  2. Per lo meno, il blocco catch dovrebbe contenere un commento che spiega perché è opportuno ignorare l'eccezione.

Un blocco catch vuoto essenzialmente sta dicendo " Non voglio sapere quali errori vengono generati, li ignorerò. "

È simile al su Error Resume Next di VB6, tranne per il fatto che qualsiasi cosa nel blocco try dopo aver generato l'eccezione verrà saltata.

Che non aiuta quando qualcosa si rompe.

Questo va di pari passo con, " Non usare eccezioni per controllare il flusso del programma. " ;, e, " Usa solo eccezioni per circostanze eccezionali. " In questo caso, si dovrebbero verificare eccezioni solo in caso di problemi. E se c'è un problema, non vuoi fallire silenziosamente. Nelle rare anomalie in cui non è necessario gestire il problema, è necessario registrare almeno l'eccezione, nel caso in cui l'anomalia non diventi più un'anomalia. L'unica cosa peggiore del fallimento è il fallimento silenzioso.

Penso che un blocco catch completamente vuoto sia una cattiva idea perché non c'è modo di dedurre che ignorare l'eccezione fosse il comportamento previsto del codice. Non è necessariamente male ingoiare un'eccezione e restituire false o null o qualche altro valore in alcuni casi. Il framework .net ha molti "provare" metodi che si comportano in questo modo. Come regola generale, se ingerisci un'eccezione, aggiungi un commento e un'istruzione di registro se l'applicazione supporta la registrazione.

Perché se viene generata un'eccezione non la vedrai mai - fallire silenziosamente è la peggiore opzione possibile - otterrai un comportamento errato e nessuna idea di guardare dove sta accadendo. Almeno metti un messaggio di log lì! Anche se è qualcosa che "non può mai accadere"!

I blocchi di cattura vuoti indicano che un programmatore non sa cosa fare con un'eccezione. Stanno sopprimendo l'eccezione dal possibile gorgogliamento e dalla corretta gestione da parte di un altro blocco try. Cerca sempre di fare qualcosa con l'eccezione che stai rilevando.

Trovo che il più fastidioso con le dichiarazioni di cattura vuote sia quando lo ha fatto qualche altro programmatore. Ciò che intendo è quando è necessario eseguire il debug del codice da qualcun altro qualsiasi dichiarazione di cattura vuota rende un'impresa del genere più difficile di quanto debba essere. Le dichiarazioni di cattura IMHO dovrebbero sempre mostrare un qualche tipo di messaggio di errore - anche se l'errore non viene gestito, dovrebbe almeno rilevarlo (alt. Attivo solo in modalità debug)

Probabilmente non è mai la cosa giusta perché stai silenziosamente passando ogni possibile eccezione. Se c'è un'eccezione specifica che ti aspetti, allora dovresti testarla, riprova se non è la tua eccezione. 

try
{
    // Do some processing.
}
catch (FileNotFound fnf)
{
    HandleFileNotFound(fnf);
}
catch (Exception e)
{
    if (!IsGenericButExpected(e))
        throw;
}

public bool IsGenericButExpected(Exception exception)
{
    var expected = false;
    if (exception.Message == "some expected message")
    {
        // Handle gracefully ... ie. log or something.
        expected = true;
    }

    return expected;
}

In generale, dovresti solo cogliere le eccezioni che puoi effettivamente gestire. Ciò significa essere il più specifici possibile quando si rilevano eccezioni. Catturare tutte le eccezioni è raramente una buona idea e ignorare tutte le eccezioni è quasi sempre una pessima idea.

Posso solo pensare ad alcuni casi in cui un blocco catch vuoto ha uno scopo significativo. Se qualunque specifica eccezione, stai rilevando è "gestito". semplicemente ritentando l'azione non sarebbe necessario fare nulla nel blocco catch. Tuttavia, sarebbe comunque una buona idea registrare il fatto che si è verificata l'eccezione.

Un altro esempio: CLR 2.0 ha cambiato il modo in cui vengono trattate le eccezioni non gestite sul thread del finalizzatore. Prima della 2.0 il processo era autorizzato a sopravvivere a questo scenario. Nell'attuale CLR il processo viene terminato in caso di un'eccezione non gestita nel thread del finalizzatore.

Tieni presente che dovresti implementare un finalizzatore solo se ne hai davvero bisogno e anche in questo caso dovresti fare il meno possibile nel finalizzatore. Ma se qualunque lavoro il tuo finalizzatore deve fare potrebbe generare un'eccezione, devi scegliere tra il minore di due mali. Vuoi chiudere l'applicazione a causa dell'eccezione non gestita? O vuoi procedere in uno stato più o meno indefinito? Almeno in teoria quest'ultimo può essere il minore dei due mali in alcuni casi. In questi casi, il blocco di cattura vuoto impedirebbe di terminare il processo.

Voglio dire, ad esempio, a volte vuoi ottenere alcune informazioni aggiuntive da qualche parte (servizio web, database) e davvero non ti importa se otterrai queste informazioni o meno. Quindi provi a ottenerlo, e se succede qualcosa, va bene, aggiungerò solo un " catch (eccezione ignorata) {} " e questo è tutto

Quindi, andando con il tuo esempio, è una cattiva idea in quel caso perché stai catturando e ignorando tutte le eccezioni. Se prendessi solo EInfoFromIrrelevantSourceNotAvailable e lo ignorassi, andrebbe bene, ma non lo sei. Stai anche ignorando ENetworkIsDown , che può essere o non essere importante. Stai ignorando ENetworkCardHasMelted e EFPUHasDecidedThatOnePlusOneIsSeventeen , che sono quasi certamente importanti.

Un blocco catch vuoto non è un problema se è impostato per catturare (e ignorare) solo le eccezioni di determinati tipi che si ritiene non siano importanti. Le situazioni in cui è una buona idea sopprimere e ignorare silenziosamente le eccezioni tutte , senza smettere di esaminarle prima per vedere se sono previste / normali / irrilevanti o meno, sono estremamente rare.

Ci sono situazioni in cui potresti usarle, ma dovrebbero essere molto rare. Le situazioni in cui potrei usarne una sono:

  • registrazione delle eccezioni; a seconda del contesto potresti invece voler pubblicare un'eccezione non gestita o un messaggio.

  • situazioni tecniche in loop, come il rendering o l'elaborazione del suono o una richiamata della casella di riepilogo, in cui il comportamento stesso dimostrerà il problema, il lancio di un'eccezione si metterà in mezzo e la registrazione dell'eccezione probabilmente causerà solo 1000 di " fallito a XXX " messaggi.

  • programmi che non possono fallire, anche se almeno dovrebbero comunque registrare qualcosa.

per la maggior parte delle applicazioni winforms, ho scoperto che è sufficiente avere una singola istruzione try per ogni input dell'utente. Uso i seguenti metodi: (AlertBox è solo un veloce wrapper MessageBox.Show) 

  public static bool TryAction(Action pAction)
  {
     try { pAction(); return true; }
     catch (Exception exception)
     {
        LogException(exception);
        return false;
     }
  }

  public static bool TryActionQuietly(Action pAction)
  {
     try { pAction(); return true; }
     catch(Exception exception)
     {
        LogExceptionQuietly(exception);
        return false;
     }
  }

  public static void LogException(Exception pException)
  {
     try
     {
        AlertBox(pException, true);
        LogExceptionQuietly(pException);
     }
     catch { }
  }

  public static void LogExceptionQuietly(Exception pException)
  {
     try { Debug.WriteLine("Exception: {0}", pException.Message); } catch { }
  }

Quindi ogni gestore di eventi può fare qualcosa del tipo: 

  private void mCloseToolStripMenuItem_Click(object pSender, EventArgs pEventArgs)
  {
     EditorDefines.TryAction(Dispose);
  }

o 

  private void MainForm_Paint(object pSender, PaintEventArgs pEventArgs)
  {
     EditorDefines.TryActionQuietly(() => Render(pEventArgs));
  }

Teoricamente, potresti avere TryActionSilently, che potrebbe essere migliore per il rendering delle chiamate in modo che un'eccezione non generi una quantità infinita di messaggi.

Non dovresti mai avere un blocco catch vuoto. È come nascondere un errore che conosci. Per lo meno, dovresti scrivere un'eccezione a un file di registro per rivederlo in seguito se viene premuto a lungo.

Se non sai cosa fare nel blocco catch, puoi semplicemente registrare questa eccezione, ma non lasciarla vuota. 

        try
        {
            string a = "125";
            int b = int.Parse(a);
        }
        catch (Exception ex)
        {
            Log.LogError(ex);
        }
Autorizzato sotto: CC-BY-SA insieme a attribuzione
Non affiliato a StackOverflow
scroll top