Warum sind leere Fangblöcke eine schlechte Idee? [geschlossen]

192

Ich habe gerade eine Frage zu Try-Catch gesehen . Welche Leute (einschließlich Jon Skeet) sagen, dass leere Fangblöcke eine wirklich schlechte Idee sind? Warum das? Gibt es keine Situation, in der ein leerer Fang keine falsche Entwurfsentscheidung ist?

Ich meine zum Beispiel, manchmal möchten Sie zusätzliche Informationen von irgendwoher erhalten (Webservice, Datenbank) und es ist Ihnen wirklich egal, ob Sie diese Informationen erhalten oder nicht. Also versuchst du es zu bekommen und wenn etwas passiert, ist das in Ordnung, ich füge einfach einen "catch (Ausnahme ignoriert) {}" hinzu und das ist alles

Samuel Carrijo
quelle
53
Ich würde einen Kommentar schreiben, der erklärt, warum es leer sein sollte.
Mehrdad Afshari
5
... oder zumindest protokollieren, dass es gefangen wurde.
Matt B
2
@ocdecio: Vermeiden Sie es für Bereinigungscode , nicht generell.
John Saunders
1
@ocdecio: Ein weiterer Fall, um die Verwendung von try..catch zu vermeiden, besteht darin, Ausnahmen für bekannte Fehlertypen abzufangen. zB numerische Konvertierungsausnahmen
MPritchard
1
@ocdecio - try..finally ist besser als try..empty catch, da der Fehler den Stapel fortsetzt - entweder vom Framework behandelt zu werden oder das Programm zu beenden und dem Benutzer angezeigt zu werden - beide Ergebnisse sind besser als ein " stiller Fehler ".
Nate

Antworten:

297

Normalerweise ist ein leerer Try-Catch eine schlechte Idee, da Sie stillschweigend einen Fehlerzustand verschlucken und dann die Ausführung fortsetzen. Gelegentlich mag dies das Richtige sein, aber oft ist es ein Zeichen dafür, dass ein Entwickler eine Ausnahme gesehen hat, nicht wusste, was er dagegen tun soll, und deshalb einen leeren Haken verwendet hat, um das Problem zum Schweigen zu bringen.

Dies ist das Programmieräquivalent zum Anbringen eines schwarzen Klebebands über einer Motorwarnleuchte.

Ich glaube, wie Sie mit Ausnahmen umgehen, hängt davon ab, auf welcher Ebene der Software Sie arbeiten: Ausnahmen im Regenwald .

Ned Batchelder
quelle
4
In den wirklich seltenen Fällen, in denen ich die Ausnahme wirklich nicht benötige und die Protokollierung aus irgendeinem Grund nicht verfügbar ist, stelle ich sicher, dass dies beabsichtigt ist - und warum es nicht benötigt wird, und bekräftige, dass ich es immer noch vorziehen würde, es zu protokollieren, wenn es war in dieser Umgebung machbar. Grundsätzlich mache ich den Kommentar MEHR Arbeit als die Protokollierung gewesen wäre, wenn es unter diesen Umständen eine Option gewesen wäre. Zum Glück habe ich nur 2 Kunden, für die dies ein Problem darstellt, und dies nur, wenn unkritische E-Mails von ihren Websites gesendet werden.
John Rudy
37
Lieben Sie auch die Analogie - und wie alle Regeln gibt es Ausnahmen. Zum Beispiel ist es vollkommen in Ordnung, schwarzes Band über der blinkenden Uhr Ihres Videorecorders zu verwenden, wenn Sie nie vorhaben, zeitgesteuerte Aufnahmen zu machen (für alle alten Leute, die sich daran erinnern, was ein Videorecorder ist).
Michael Burr
3
Wie bei jeder Abweichung von der akzeptierten Praxis tun Sie dies gegebenenfalls und dokumentieren Sie es, damit der nächste Mann weiß, was Sie getan haben und warum.
David Thornley
5
@Jason: Zumindest sollten Sie einen detaillierten Kommentar hinzufügen, der erklärt, warum Sie die Ausnahmen zum Schweigen bringen.
Ned Batchelder
1
Diese Antwort setzt zwei Dinge voraus: 1. Dass die ausgelöste Ausnahme tatsächlich eine sinnvolle ist und dass derjenige, der den Code geschrieben hat, der wirft, wusste, was er tat; 2. Dass die Ausnahme tatsächlich eine "Fehlerbedingung" ist und nicht als Kontrollfluss missbraucht wird (obwohl dies ein spezifischerer Fall meines ersten Punktes ist).
Boris B.
37

Sie sind im Allgemeinen eine schlechte Idee, da es sich um eine wirklich seltene Erkrankung handelt, bei der ein Fehler (Ausnahmezustand, allgemeiner) ordnungsgemäß ohne jegliche Reaktion behandelt wird. Darüber hinaus sind leere catchBlöcke ein gängiges Tool, das von Personen verwendet wird, die die Ausnahme-Engine zur Fehlerprüfung verwenden, um sicherzustellen, dass sie dies präventiv tun sollten.

Zu sagen, dass es immer schlecht ist, ist falsch ... das gilt für sehr wenig. Es kann Umstände geben, in denen es Ihnen entweder egal ist, dass ein Fehler aufgetreten ist, oder dass das Vorhandensein des Fehlers irgendwie darauf hinweist, dass Sie sowieso nichts dagegen tun können (z. B. wenn Sie einen vorherigen Fehler in eine Textprotokolldatei schreiben und Sie erhalten eine IOException, was bedeutet, dass Sie den neuen Fehler sowieso nicht ausschreiben konnten.

Adam Robinson
quelle
11

Ich würde die Dinge nicht so weit strecken, dass jemand, der leere Fangblöcke verwendet, ein schlechter Programmierer ist und nicht weiß, was er tut ...

Bei Bedarf verwende ich leere Fangblöcke. Manchmal weiß der Programmierer der Bibliothek, die ich konsumiere, nicht, was er tut, und löst Ausnahmen aus, selbst in Situationen, in denen niemand sie benötigt.

Betrachten Sie zum Beispiel eine http-Serverbibliothek. Es ist mir egal, ob der Server eine Ausnahme auslöst, da der Client die Verbindung getrennt hat und index.htmlnicht gesendet werden konnte.

lubos hasko
quelle
6
Sie verallgemeinern sicherlich. Nur weil Sie es nicht brauchen, heißt das nicht, dass es niemand tut. Selbst wenn als Antwort absolut nichts getan werden kann, gibt es irgendwo jemanden, der Statistiken über abgebrochene Verbindungen sammeln muss. Es ist also ziemlich unhöflich anzunehmen, dass "der Programmierer der Bibliothek nicht weiß, was er tut".
Ben Voigt
8

Es gibt seltene Fälle, in denen dies gerechtfertigt sein kann. In Python sieht man oft diese Art von Konstruktion:

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

Es kann also in Ordnung sein (abhängig von Ihrer Anwendung), Folgendes zu tun:

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 einem kürzlich durchgeführten .NET-Projekt musste ich Code schreiben, um Plugin-DLLs aufzulisten und Klassen zu finden, die eine bestimmte Schnittstelle implementieren. Das relevante Codebit (in VB.NET, sorry) ist:

    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

Obwohl ich selbst in diesem Fall zugeben würde, dass das Protokollieren des Fehlers irgendwo eine Verbesserung wäre.

Daniel Pryden
quelle
Ich habe log4net als eine dieser Instanzen bezeichnet, bevor ich Ihre Antwort sah.
Scott Lawrence
7

Leere Fangblöcke werden normalerweise eingefügt, weil der Codierer nicht wirklich weiß, was sie tun. In meiner Organisation muss ein leerer Fangblock einen Kommentar enthalten, warum es eine gute Idee ist, mit der Ausnahme nichts zu tun.

In einem ähnlichen Zusammenhang wissen die meisten Leute nicht, dass auf einen try {} -Block entweder ein catch {} oder ein finally {} folgen kann, nur einer ist erforderlich.

Justin
quelle
1
+1 Ich würde das 'oder' in diesem letzten Satz für zusätzliche Wirkung
betonen
Dieser zweite Absatz kann jedoch sprachabhängig sein, oder?
David Z
3
es sei denn natürlich reden Sie über IE6 oder IE7 ;-) ... wenn der Fang IS erforderlich oder die schließlich nicht ausgeführt. (Dies wurde übrigens in IE8 behoben)
Scunliffe
Sehr richtig. Könnte es wert sein, die Sprachen hervorzuheben. Ich glaube, .net ist in Ordnung
MPritchard
7

Ausnahmen sollten nur ausgelöst werden, wenn es wirklich eine Ausnahme gibt - etwas, das über die Norm hinausgeht. Ein leerer Fangblock sagt im Grunde: "Es passiert etwas Schlimmes, aber es ist mir einfach egal." Das ist eine schlechte Idee.

Wenn Sie die Ausnahme nicht behandeln möchten, lassen Sie sie nach oben übertragen, bis ein Code erreicht ist, der sie verarbeiten kann. Wenn nichts mit der Ausnahme umgehen kann, sollte die Anwendung heruntergefahren werden.

Reed Copsey
quelle
3
Manchmal weiß man, dass es nichts beeinflusst. Dann mach es und kommentiere es, damit der nächste nicht nur denkt, dass du es vermasselt hast, weil du inkompetent bist.
David Thornley
Ja, es gibt eine Zeit und einen Ort für alles. Im Allgemeinen denke ich jedoch, dass ein leerer Catch-Block, der gut ist, die Ausnahme von der Regel ist - es ist ein seltener Fall, in dem Sie eine Ausnahme wirklich ignorieren möchten (normalerweise, IMO, ist es das Ergebnis einer schlechten Verwendung von Ausnahmen).
Reed Copsey
2
@ David Thornley: Woher weißt du, dass es nichts beeinflusst, wenn du die Ausnahme nicht zumindest prüfst, um festzustellen, ob es sich um einen erwarteten und bedeutungslosen Fehler handelt oder um einen Fehler, der stattdessen nach oben weitergegeben werden sollte?
Dave Sherohman
2
@ Dave: Während catch (Exception) {}ist eine schlechte Idee, catch (SpecificExceptionType) {}könnte vollkommen in Ordnung sein. Der Programmierer hat die Ausnahme anhand der Typinformationen in der catch-Klausel überprüft.
Ben Voigt
7

Ich denke, es ist in Ordnung, wenn Sie einen bestimmten Ausnahmetyp abfangen, von dem Sie wissen, dass er nur aus einem bestimmten Grund ausgelöst wird, und Sie erwarten diese Ausnahme und müssen wirklich nichts dagegen tun.

Aber auch in diesem Fall ist möglicherweise eine Debug-Meldung angebracht.

balpha
quelle
5

Per Josh Bloch - Punkt 65: Ausnahmen von effektivem Java nicht ignorieren :

  1. Ein leerer Fangblock macht den Zweck von Ausnahmen zunichte
  2. Zumindest sollte der catch-Block einen Kommentar enthalten, der erklärt, warum es angebracht ist, die Ausnahme zu ignorieren.
KrishPrabakar
quelle
3

Ein leerer Fangblock sagt im Wesentlichen: "Ich möchte nicht wissen, welche Fehler ausgelöst werden, ich werde sie einfach ignorieren."

Es ist ähnlich wie bei VB6 On Error Resume Next, außer dass alles im try-Block, nachdem die Ausnahme ausgelöst wurde, übersprungen wird.

Was nicht hilft, wenn dann etwas kaputt geht.

Powerlord
quelle
Ich würde tatsächlich sagen, dass "On Error Resume Next" schlechter ist - es wird trotzdem nur die nächste Zeile versucht. Ein leerer Fang wird springen, um die schließende Klammer der try-Anweisung zu passieren
MPritchard
Guter Punkt. Ich sollte das wahrscheinlich in meiner Antwort beachten.
Powerlord
1
Dies ist nicht genau richtig, wenn Sie einen leeren Versuch haben ... mit einem bestimmten Ausnahmetyp zu fangen, dann wird nur dieser Ausnahmetyp ignoriert, und das könnte legitim sein ...
Meta-Knight
Wenn Sie jedoch wissen, dass eine bestimmte Art von Ausnahme ausgelöst wird, haben Sie möglicherweise genügend Informationen, um Ihre Logik so zu schreiben, dass die Verwendung von try-catch zur Bewältigung der Situation vermieden wird.
Scott Lawrence
@Scott: Bestimmte Sprachen (z. B. Java) haben Ausnahmen überprüft ... Ausnahmen, für die Sie Catch-Blöcke schreiben müssen.
Powerlord
3

Dies geht Hand in Hand mit "Verwenden Sie keine Ausnahmen zur Steuerung des Programmflusses" und "Verwenden Sie Ausnahmen nur für außergewöhnliche Umstände". In diesem Fall sollten Ausnahmen nur auftreten, wenn ein Problem vorliegt. Und wenn es ein Problem gibt, möchten Sie nicht stillschweigend scheitern. In den seltenen Anomalien, in denen das Problem nicht behoben werden muss, sollten Sie zumindest die Ausnahme protokollieren, nur für den Fall, dass die Anomalie nicht mehr zu einer Anomalie wird. Das einzige, was schlimmer ist als zu scheitern, ist lautlos zu scheitern.

Imagist
quelle
2

Ich denke, ein vollständig leerer Catch-Block ist eine schlechte Idee, da es keine Möglichkeit gibt, darauf zu schließen, dass das Ignorieren der Ausnahme das beabsichtigte Verhalten des Codes war. Es ist nicht unbedingt schlecht, eine Ausnahme zu schlucken und in einigen Fällen false oder null oder einen anderen Wert zurückzugeben. Das .net-Framework verfügt über viele "try" -Methoden, die sich so verhalten. Wenn Sie eine Ausnahme verschlucken, sollten Sie als Faustregel einen Kommentar und eine Protokollanweisung hinzufügen, wenn die Anwendung die Protokollierung unterstützt.

komplexe Chiffre
quelle
1

Denn wenn eine Ausnahme ausgelöst wird , werden Sie sie nie sehen - ein stillschweigendes Versagen ist die schlechteste Option - Sie erhalten ein fehlerhaftes Verhalten und keine Ahnung, wo es passiert. Zumindest eine Protokollnachricht dort setzen! Auch wenn es etwas ist, das niemals passieren kann!

Nate
quelle
1

Leere Fangblöcke sind ein Hinweis darauf, dass ein Programmierer mit einer Ausnahme nicht weiß, was er tun soll. Sie unterdrücken, dass die Ausnahme möglicherweise sprudelt und von einem anderen Try-Block korrekt behandelt wird. Versuchen Sie immer, etwas zu tun, mit der Ausnahme, die Sie fangen.

Dan
quelle
1

Ich finde es am ärgerlichsten, wenn leere catch-Anweisungen dies tun, wenn es ein anderer Programmierer getan hat. Ich meine, wenn Sie Code von jemand anderem debuggen müssen, erschweren leere catch-Anweisungen ein solches Unterfangen, als es sein muss. IMHO catch-Anweisungen sollten immer eine Art Fehlermeldung anzeigen - auch wenn der Fehler nicht behandelt wird, sollte er ihn zumindest erkennen (alt. On nur im Debug-Modus)

AndersK
quelle
1

Es ist wahrscheinlich nie das Richtige, weil Sie stillschweigend jede mögliche Ausnahme bestehen. Wenn Sie eine bestimmte Ausnahme erwarten, sollten Sie sie testen und erneut auslösen, wenn dies nicht Ihre Ausnahme ist.

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;
}
xanadont
quelle
1
Das ist nicht wirklich ein Standardmuster, das Sie den Leuten raten, dort zu verwenden. Die meisten Leute würden nur die Ausnahmen der Arten erfassen, die sie erwarten, und nicht die, die sie nicht sind. Ich kann einige Umstände sehen, unter denen Ihr Ansatz gültig wäre. Achten Sie nur darauf, dass Sie nicht in einem Forum wie diesem posten, in dem Leute dazu neigen, solche Dinge zu lesen, weil sie sie überhaupt nicht verstehen.)
MPritchard
Ich wollte nicht, dass IsKnownException () den Typ der Ausnahme überprüft (ja, das sollten Sie mit verschiedenen Catch-Blöcken tun), sondern ob es sich um eine erwartete Ausnahme handelt. Ich werde bearbeiten, um es klarer zu machen.
Xanadont
Ich habe ursprünglich über COMExceptions nachgedacht. Sie können auf einen bestimmten Fehlercode testen und die erwarteten Codes verarbeiten. Ich mache das oft beim Programmieren mit ArcOjbects.
Xanadont
2
-1 Entscheidungen im Code basierend auf der Ausnahmemeldung zu treffen, ist eine wirklich schlechte Idee. Die genaue Nachricht ist selten dokumentiert und kann sich jederzeit ändern. Es ist ein Implementierungsdetail. Oder die Nachricht könnte auf einer lokalisierbaren Ressourcenzeichenfolge basieren. Auf jeden Fall soll es nur von einem Menschen gelesen werden.
Wim Coenen
Eek. Die Ausnahme. Nachricht könnte lokalisiert sein und somit nicht das, was Sie erwarten. Das ist einfach falsch.
Frankhommers
1

Im Allgemeinen sollten Sie nur die Ausnahmen abfangen, die Sie tatsächlich behandeln können. Das bedeutet, beim Abfangen von Ausnahmen so genau wie möglich zu sein. Das Abfangen aller Ausnahmen ist selten eine gute Idee und das Ignorieren aller Ausnahmen ist fast immer eine sehr schlechte Idee.

Ich kann mir nur einige Fälle vorstellen, in denen ein leerer Fangblock einen sinnvollen Zweck hat. Wenn eine bestimmte Ausnahme, die Sie abfangen, durch erneutes Ausprobieren der Aktion "behandelt" wird, muss im catch-Block nichts unternommen werden. Es wäre jedoch immer noch eine gute Idee, die Tatsache zu protokollieren, dass die Ausnahme aufgetreten ist.

Ein weiteres Beispiel: CLR 2.0 hat die Behandlung nicht behandelter Ausnahmen im Finalizer-Thread geändert. Vor 2.0 durfte der Prozess dieses Szenario überleben. In der aktuellen CLR wird der Prozess im Falle einer nicht behandelten Ausnahme im Finalizer-Thread beendet.

Denken Sie daran, dass Sie einen Finalizer nur implementieren sollten, wenn Sie wirklich einen benötigen, und selbst dann sollten Sie im Finalizer so wenig wie möglich tun. Aber wenn die Arbeit, die Ihr Finalizer leisten muss, eine Ausnahme auslösen könnte, müssen Sie zwischen dem kleineren von zwei Übeln wählen. Möchten Sie die Anwendung aufgrund der nicht behandelten Ausnahme herunterfahren? Oder möchten Sie in einem mehr oder weniger undefinierten Zustand fortfahren? Zumindest theoretisch kann letzteres in einigen Fällen das kleinere von zwei Übeln sein. In diesem Fall würde der leere Fangblock verhindern, dass der Prozess beendet wird.

Brian Rasmussen
quelle
1
Ich meine zum Beispiel, manchmal möchten Sie zusätzliche Informationen von irgendwoher erhalten (Webservice, Datenbank) und es ist Ihnen wirklich egal, ob Sie diese Informationen erhalten oder nicht. Also versuchst du es zu bekommen und wenn etwas passiert, ist das in Ordnung, ich füge einfach einen "catch (Ausnahme ignoriert) {}" hinzu und das ist alles

Wenn Sie also Ihrem Beispiel folgen, ist dies in diesem Fall eine schlechte Idee, da Sie alle Ausnahmen abfangen und ignorieren . Wenn Sie nur fangen EInfoFromIrrelevantSourceNotAvailableund es ignorieren würden, wäre das in Ordnung, aber Sie sind es nicht. Sie ignorieren auch ENetworkIsDown, was wichtig sein kann oder nicht. Sie ignorieren ENetworkCardHasMeltedund EFPUHasDecidedThatOnePlusOneIsSeventeen, die mit ziemlicher Sicherheit wichtig sind.

Ein leerer catch-Block ist kein Problem, wenn er so eingerichtet ist, dass nur Ausnahmen bestimmter Typen abgefangen (und ignoriert) werden, von denen Sie wissen, dass sie unwichtig sind. Die Situationen, in denen es eine gute Idee ist, alle Ausnahmen zu unterdrücken und stillschweigend zu ignorieren , ohne sie zuerst zu untersuchen, um festzustellen, ob sie erwartet / normal / irrelevant sind oder nicht, sind äußerst selten.

Dave Sherohman
quelle
1

Es gibt Situationen, in denen Sie sie verwenden könnten, aber sie sollten sehr selten sein. Zu den Situationen, in denen ich eine verwenden könnte, gehören:

  • Ausnahmeprotokollierung; Je nach Kontext möchten Sie möglicherweise stattdessen eine nicht behandelte Ausnahme oder Nachricht.

  • Das Schleifen technischer Situationen wie Rendering oder Soundverarbeitung oder ein Listbox-Rückruf, bei dem das Verhalten selbst das Problem demonstriert, das Auslösen einer Ausnahme im Weg steht und das Protokollieren der Ausnahme wahrscheinlich nur zu Tausenden von "Fehlgeschlagen an XXX" -Nachrichten führt .

  • Programme, die nicht fehlschlagen können, obwohl sie zumindest noch etwas protokollieren sollten.

Für die meisten Winforms-Anwendungen habe ich festgestellt, dass es ausreicht, für jede Benutzereingabe eine einzige try-Anweisung zu haben. Ich verwende die folgenden Methoden: (AlertBox ist nur ein schneller MessageBox.Show-Wrapper)

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

Dann kann jeder Event-Handler Folgendes tun:

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

oder

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

Theoretisch könnten Sie TryActionSilently verwenden, das möglicherweise besser zum Rendern von Anrufen geeignet ist, damit eine Ausnahme nicht unendlich viele Nachrichten generiert.

Dave Cousineau
quelle
1

Wenn Sie nicht wissen, was Sie im catch-Block tun sollen, können Sie diese Ausnahme einfach protokollieren, aber nicht leer lassen.

        try
        {
            string a = "125";
            int b = int.Parse(a);
        }
        catch (Exception ex)
        {
            Log.LogError(ex);
        }
Andzej Maciusovic
quelle
Warum wurde das abgelehnt?
Vino
0

Sie sollten niemals einen leeren Fangblock haben. Es ist, als würde man einen Fehler verstecken, von dem man weiß. Zumindest sollten Sie eine Ausnahme in eine Protokolldatei schreiben, um sie später zu überprüfen, wenn Sie unter Zeitdruck stehen.

Chadit
quelle