Ist `catch (…) {throw; } `eine schlechte Praxis?

74

Ich stimme zwar zu, dass das Fangen ... ohne erneutes Werfen in der Tat falsch ist, aber ich glaube, dass die Verwendung von Konstrukten wie folgt :

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

Ist in Fällen akzeptabel, in denen RAII nicht anwendbar ist . (Bitte fragen Sie nicht ... nicht jeder in meiner Firma mag objektorientiertes Programmieren und RAII wird oft als "nutzloses Schulmaterial" angesehen ...)

Meine Kollegen sagen, dass Sie immer wissen sollten, welche Ausnahmen geworfen werden sollen und dass Sie immer Konstrukte verwenden können wie:

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

Gibt es eine allgemein anerkannte gute Praxis in Bezug auf diese Situationen?

ereOn
quelle
3
@Pubby: Ich bin mir nicht sicher, ob dies genau dieselbe Frage ist. Die verknüpfte Frage bezieht sich eher auf "Sollte ich fangen ...", während sich meine Frage auf "Soll ich besser fangen ...oder <specific exception>bevor ich erneut wirf" konzentriert
bevor es
53
Tut mir leid, das zu sagen, aber C ++ ohne RAII ist nicht C ++.
Fredoverflow
46
Also lehnen Ihre Kuharbeiter die Technik ab, die erfunden wurde, um ein bestimmtes Problem zu lösen, und diskutieren dann, welche der minderwertigen Alternativen verwendet werden sollte? Tut mir leid zu sagen, aber das scheint dumm zu sein , egal wie ich es betrachte.
sbi
11
"Fangen ... ohne erneutes Werfen ist in der Tat falsch" - Sie irren sich. In main, catch(...) { return EXIT_FAILURE; }könnte auch richtig sein in Code, der nicht unter einem Debugger ausgeführt wird . Wenn Sie nicht fangen, wird der Stapel möglicherweise nicht abgewickelt. Nur wenn Ihr Debugger nicht erfasste Ausnahmen erkennt, möchten Sie, dass sie beendet werden main.
Steve Jessop
3
... selbst wenn es sich um einen "Programmierfehler" handelt, folgt daraus nicht unbedingt, dass Sie nichts darüber wissen möchten. Wie auch immer, Ihre Kollegen sind keine guten Softwareprofis, und wie sbi sagt, ist es sehr schwierig, darüber zu sprechen, wie man am besten mit einer Situation umgeht, die anfangs chronisch schwach ist.
Steve Jessop

Antworten:

196

Meine Kollegen sagen, dass Sie immer wissen sollten, welche Ausnahmen geworfen werden sollen [...]

Ihr Kollege hat offenbar noch nie an Universalbibliotheken gearbeitet.

Wie in der Welt kann eine Klasse wie std::vectorauch vorgeben zu wissen , was die Kopierkonstruktoren werfen, während sie noch Ausnahme Sicherheit zu gewährleisten?

Wenn Sie immer wüssten, was der Angerufene beim Kompilieren tun würde, wäre Polymorphismus nutzlos! Manchmal besteht das gesamte Ziel darin, das zu abstrahieren, was auf einer niedrigeren Ebene passiert. Sie möchten also nicht genau wissen, was los ist.

Mehrdad
quelle
32
Eigentlich, auch wenn sie wüssten, dass Ausnahmen geworfen werden sollen. Was ist der Zweck dieser Code-Vervielfältigung? Sofern sich die Handhabung nicht unterscheidet, sehe ich keinen Grund darin, die Ausnahmen aufzuzählen, um Ihr Wissen zu demonstrieren.
Michael Krelin - Hacker
3
@ MichaelKrelin-Hacker: Das auch. Hinzu kommt, dass die Ausnahmespezifikationen abgelehnt wurden, da das Auflisten aller möglichen Ausnahmen im Code später zu Fehlern führte ... das ist die schlimmste Idee, die es je gab.
Mehrdad
4
Und was mich stört, ist, was der Ursprung einer solchen Einstellung sein könnte, wenn ich nützliche und bequeme Techniken als "nutzloses Schulmaterial" ansähe. Aber gut ...
Michael Krelin - Hacker
1
+1, die Aufzählung aller möglichen Optionen ist ein hervorragendes Rezept für einen zukünftigen Misserfolg ....
Littleadv
2
Gute Antwort. Es könnte möglicherweise von Vorteil sein zu erwähnen, dass es nicht klug ist, Funktionen aus Bereich X zu verwenden, wenn ein Compiler, den man unterstützen muss, einen Fehler in Bereich X aufweist, zumindest nicht direkt. Zum Beispiel wäre ich angesichts der Informationen über das Unternehmen nicht überrascht, wenn sie Visual C ++ 6.0 verwenden würden, das einige dumme Fehler in diesem Bereich aufweist (wie zum Beispiel, dass Objektdestruktoren für Ausnahmen zweimal aufgerufen werden) - einige kleinere Nachkommen dieser frühen Fehler haben überlebt an diesem Tag, aber erfordern sorgfältige Vorkehrungen zu manifestieren.
Alf P. Steinbach
44

Was Sie gefangen zu sein scheinen, ist die Hölle, in der jemand versucht, seinen Kuchen zu haben und ihn auch zu essen.

RAII und Ausnahmen sollen Hand in Hand gehen. RAII ist das Mittel, mit dem Sie nicht viele Anweisungen schreiben müssen , um catch(...)aufzuräumen. Dies geschieht selbstverständlich automatisch. Und Ausnahmen sind die einzige Möglichkeit, mit RAII-Objekten zu arbeiten, da Konstruktoren nur erfolgreich sein oder werfen können (oder das Objekt in einen Fehlerzustand versetzen können, aber wer will das?).

Eine catchAnweisung kann eine der beiden folgenden Aufgaben ausführen: Fehler oder außergewöhnliche Umstände behandeln oder Aufräumarbeiten ausführen. Manchmal macht es beides, aber es gibt jede catchAnweisung, um mindestens eine davon zu tun.

catch(...)kann keine ordnungsgemäße Ausnahmebehandlung durchführen. Sie wissen nicht, was die Ausnahme ist. Sie können keine Informationen über die Ausnahme erhalten. Sie haben absolut keine andere Information als die Tatsache, dass eine Ausnahme von etwas in einem bestimmten Codeblock ausgelöst wurde . Das einzig legitime, was Sie in einem solchen Block tun können, ist aufzuräumen. Und das bedeutet, die Ausnahme am Ende der Bereinigung erneut auszulösen.

Was RAII Ihnen in Bezug auf die Ausnahmebehandlung bietet, ist die kostenlose Bereinigung. Wenn alles ordnungsgemäß in RAII eingekapselt ist, wird alles ordnungsgemäß bereinigt. catchAnweisungen müssen nicht mehr bereinigt werden. In diesem Fall gibt es keinen Grund, eine catch(...)Erklärung abzugeben.

Also würde ich zustimmen, dass catch(...)das meistens böse ist ... vorläufig .

Diese Bestimmung ist die ordnungsgemäße Verwendung von RAII. Denn ohne sie, Sie müssen bestimmte Bereinigungen tun zu können. Daran führt kein Weg vorbei. Sie müssen in der Lage sein, Aufräumarbeiten zu erledigen. Sie müssen sicherstellen können, dass durch das Auslösen einer Ausnahme der Code in einem angemessenen Zustand bleibt. Und catch(...)ist dabei ein wichtiges Instrument.

Sie können nicht eins ohne das andere haben. Man kann nicht sagen, dass sowohl RAII als catch(...) auch schlecht sind. Sie benötigen mindestens eines davon. Andernfalls sind Sie nicht ausnahmesicher.

Natürlich gibt es eine gültige, wenn auch seltene Verwendung catch(...), die nicht einmal RAII verbannen kann: die exception_ptrWeiterleitung an eine andere Person. In der Regel über eine promise/futureoder eine ähnliche Schnittstelle.

Meine Kollegen sagen, dass Sie immer wissen sollten, welche Ausnahmen geworfen werden sollen und dass Sie immer Konstrukte verwenden können wie:

Ihr Mitarbeiter ist ein Idiot (oder einfach nur schrecklich unwissend). Dies sollte sofort offensichtlich sein, da er vorschlägt, wie viel Code zum Kopieren und Einfügen Sie schreiben. Die Bereinigung für jede dieser catch-Anweisungen ist genau gleich . Das ist ein Wartungs-Albtraum, von der Lesbarkeit ganz zu schweigen.

Kurz gesagt: Dies ist das Problem, für dessen Lösung RAII erstellt wurde (nicht, dass es andere Probleme nicht löst).

Was mich an dieser Vorstellung verwirrt, ist, dass es im Allgemeinen rückständig ist, wie die meisten Leute behaupten, dass RAII schlecht ist. Im Allgemeinen lautet das Argument "RAII ist schlecht, weil Sie Ausnahmen verwenden müssen, um Konstruktorfehler zu signalisieren. Sie können jedoch keine Ausnahmen auslösen, da es nicht sicher ist und Sie viele catchAnweisungen benötigen, um alles zu bereinigen." Das ist ein kaputtes Argument, weil RAII das Problem löst , das der Mangel an RAII verursacht.

Höchstwahrscheinlich ist er gegen RAII, weil es Details verbirgt. Destruktor-Aufrufe sind bei automatischen Variablen nicht sofort sichtbar. Sie erhalten also Code, der implizit aufgerufen wird. Einige Programmierer hassen das wirklich. Anscheinend catchist es eine bessere Idee , drei Anweisungen zu haben, die alle dasselbe mit Code zum Kopieren und Einfügen tun.

Nicol Bolas
quelle
2
Anscheinend schreiben Sie keinen Code, der eine starke Ausnahmesicherheitsgarantie bietet . RAII hilft bei der Bereitstellung von Grundgarantie. Um jedoch eine starke Garantie zu gewährleisten, müssen Sie einige Aktionen rückgängig machen, um das System auf den Zustand zurückzusetzen, den es vor dem Aufrufen der Funktion hatte. Grundlegende Garantie ist Bereinigung , starke Garantie ist Rollback . Das Rollback ist funktionsspezifisch. Man kann es also nicht in "RAII" setzen. Und dann wird der Catch-All- Block praktisch. Wenn Sie Code mit starker Garantie schreiben, verwenden Sie häufig catch-all .
anton_rh
@anton_rh: Vielleicht, aber auch in diesen Fällen sind Catch-All-Anweisungen das Werkzeug des letzten Auswegs . Das bevorzugte Werkzeug ist, alles zu tun, was ausgelöst wird, bevor Sie einen Zustand ändern, den Sie im Ausnahmefall wiederherstellen müssten. Natürlich können Sie nicht in allen Fällen alles auf diese Weise implementieren, aber dies ist der ideale Weg, um die starke Ausnahmegarantie zu erhalten.
Nicol Bolas
14

Wirklich zwei Kommentare. Das erste ist, dass Sie in einer idealen Welt immer wissen sollten, welche Ausnahmen in der Praxis möglicherweise ausgelöst werden, wenn Sie mit Bibliotheken von Drittanbietern oder mit einem Microsoft-Compiler arbeiten. Mehr auf den Punkt; Auch wenn Sie alle möglichen Ausnahmen genau kennen, ist das hier relevant? catch (...)drückt die Absicht weitaus besser aus als catch ( std::exception const& ), selbst wenn alle möglichen Ausnahmen davon herrühren std::exception(was in einer idealen Welt der Fall wäre). Was die Verwendung mehrerer Fangblöcke angeht, wenn es keine gemeinsame Grundlage für alle Ausnahmen gibt: Das ist völlige Verschleierung und ein Alptraum für die Instandhaltung. Woran erkennt man, dass alle Verhaltensweisen identisch sind? Und das war die Absicht? Und was passiert, wenn Sie das Verhalten ändern müssen (z. B. Fehlerbehebung)? Es ist allzu leicht, einen zu verpassen.


quelle
3
Eigentlich hat mein Kollege seine eigene Ausnahmeklasse entworfen, die nicht von std::exceptionunserer Codebasis abstammt und jeden Tag versucht, ihre Verwendung durchzusetzen. Ich vermute, dass er versucht, mich für die Verwendung von Code und externen Bibliotheken zu bestrafen, die er nicht selbst geschrieben hat.
ereOn
17
@ereOn Es hört sich für mich so an, als ob Ihr Kollege dringend geschult werden muss. Auf jeden Fall würde ich wahrscheinlich die Verwendung von Bibliotheken vermeiden, die von ihm geschrieben wurden.
2
Vorlagen und das Wissen, welche Ausnahmen geworfen werden, gehören zusammen wie Erdnussbutter und tote Geckos. Etwas so Einfaches wie das std::vector<>kann jede Art von Ausnahme aus so ziemlich jedem Grund auslösen.
David Thornley
3
Sagen Sie uns bitte genau, woher Sie wissen, welche Ausnahme von der morgigen Fehlerbehebung weiter unten im Aufrufbaum ausgelöst wird.
Mattnz
11

Ich denke, Ihr Kollege hat einige gute Ratschläge vertauscht - Sie sollten bekannte Ausnahmen nur in einem catchBlock behandeln, wenn Sie sie nicht erneut werfen.

Das heisst:

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

Ist schlecht, weil es jeden Fehler still versteckt .

Jedoch:

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

Ist in Ordnung - wir wissen, womit wir es zu tun haben und müssen es nicht dem aufrufenden Code aussetzen.

Gleichfalls:

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

Ist in Ordnung, sogar Best Practice, sollte der Code für allgemeine Fehler mit dem Code übereinstimmen, der sie verursacht. Es ist besser, als sich auf den Angerufenen zu verlassen, um zu wissen, dass eine Transaktion rückgängig gemacht werden muss oder was auch immer.

Keith
quelle
9

Jede Antwort mit Ja oder Nein sollte mit einer Begründung versehen sein, warum dies so ist.

Zu sagen, dass es falsch ist, nur weil mir das beigebracht wurde, ist blinder Fanatismus.

Das gleiche //Some cleanup; throwmehrmals zu schreiben , wie in Ihrem Beispiel, ist falsch, da es sich um Codeduplizierung handelt und dies eine Wartungslast darstellt. Es ist besser, es nur einmal zu schreiben.

Das Schreiben von a catch(...), um alle Ausnahmen zum Schweigen zu bringen, ist falsch, da Sie nur mit Ausnahmen umgehen sollten, die Sie zu behandeln wissen. Mit diesem Platzhalter können Sie mehr als erwartet kreuzen und wichtige Fehler zum Schweigen bringen.

Wenn Sie jedoch nach a erneut werfen catch(...), gilt das letztere nicht mehr, da Sie die Ausnahme nicht tatsächlich behandeln. Es gibt also keinen Grund, warum dies nicht empfohlen wird.

Eigentlich habe ich das gemacht, um sensible Funktionen ohne Probleme einzuloggen:

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");
}
pestophag
quelle
2
Hoffen wir, dass wir Log(...)nicht werfen können.
Deduplizierer
2

Generell stimme ich der Stimmung der Posts hier zu, ich mag es nicht, bestimmte Ausnahmen zu erfassen - ich denke, die Syntax dafür steckt noch in den Kinderschuhen und kann den redundanten Code noch nicht verarbeiten.

Aber da alle das sagen, werde ich darauf hinweisen, dass ich, obwohl ich sie sparsam benutze, oft eine meiner "catch (Exception e)" - Anweisungen angesehen und gesagt habe: "Verdammt, ich wünschte, ich hätte angerufen "Wenn Sie später eintreten, ist es oft schön zu wissen, was die Absicht war und was der Kunde wahrscheinlich auf einen Blick werfen wird.

Ich rechtfertige die Einstellung von "Immer x verwenden" nicht, sondern sage nur, dass es gelegentlich schön ist, sie aufgelistet zu sehen, und ich bin sicher, dass dies der Grund ist, warum manche Leute meinen, es sei der "richtige" Weg.

Bill K
quelle