Ist die Verwendung der finally-Klausel für die Erledigung von Arbeiten nach der Rückgabe stilsicher / gefährlich?

30

Beim Schreiben eines Iterators habe ich festgestellt, dass ich den folgenden Code geschrieben habe (Fehlerbehandlung beim Entfernen).

public T next() {
  try {
    return next;
  } finally {
    next = fetcher.fetchNext(next);
  }
}

finde es etwas leichter zu lesen als

public T next() {
  T tmp = next;
  next = fetcher.fetchNext(next);
  return tmp;
}

Ich weiß, es ist ein einfaches Beispiel, bei dem der Unterschied in der Lesbarkeit vielleicht nicht so überwältigend ist, aber ich bin an der allgemeinen Meinung interessiert, ob es schlecht ist, try-finally in Fällen wie diesem zu verwenden, in denen es keine Ausnahmen gibt, oder wenn es tatsächlich bevorzugt wird, wenn es den Code vereinfacht.

Wenn es schlecht ist: warum? Stil, Leistung, Fallstricke, ...?

Fazit Vielen Dank für all Ihre Antworten! Ich denke, die Schlussfolgerung (zumindest für mich) ist, dass das erste Beispiel besser lesbar gewesen wäre, wenn es ein allgemeines Muster gewesen wäre, aber das ist es nicht. Daher überwiegt die Verwirrung, die durch die Verwendung eines Konstrukts außerhalb seines Verwendungszwecks zusammen mit einem möglicherweise verschleierten Ausnahmefluss entsteht, jegliche Vereinfachungen.

Halle Knast
quelle
10
Ich persönlich würde den zweiten mehr verstehen können als den ersten, aber ich benutze selten finallyBlöcke.
Christian Mann
2
return current = fetcher.fetchNext (current); // Wie wäre es damit, oder brauchen Sie wirklich ein Prefetching?
scarfridge
1
@scarfridge Im Beispiel geht es um die Implementierung Iterator, bei der Sie tatsächlich eine Art Vorablesezugriff benötigen, um hasNext()arbeiten zu können. Versuch es selber.
Maaartinus
1
@ Maaartinus Ich bin mir dessen bewusst. Manchmal gibt hasNext () einfach true zurück, z. B. Hagelkörnersequenz, und manchmal ist das Abrufen des nächsten Elements unerschwinglich teuer. Im letzteren Fall sollten Sie das Element nur bei Bedarf abrufen, ähnlich wie bei einer verzögerten Initialisierung.
Scarfridge
1
@scarfridge Das Problem bei der allgemeinen Verwendung von Iteratorist, dass Sie den Wert in abrufen müssen hasNext()(weil das Abrufen des Werts oft die einzige Möglichkeit ist, herauszufinden, ob er vorhanden ist) und ihn next()wie im OP zurückgeben müssen.
Maaartinus

Antworten:

18

Persönlich würde ich mir Gedanken über die Trennung von Befehlsabfragen machen . Wenn Sie gleich weitermachen, hat next () zwei Zwecke: den angekündigten Zweck, das Element next abzurufen, und den versteckten Nebeneffekt, den internen Zustand von next zu verändern. Was Sie also tun, ist, den angekündigten Zweck im Hauptteil der Methode auszuführen und dann einen versteckten Nebeneffekt in einer finally-Klausel anzugehen, was ... umständlich, wenn auch nicht "falsch", genau zu sein scheint.

Worauf es wirklich ankommt, ist, wie verständlich der Code ist, würde ich sagen, und in diesem Fall lautet die Antwort "meh". Sie "versuchen" eine einfache return-Anweisung und führen dann einen versteckten Nebeneffekt in einem Codeblock aus, der für die ausfallsichere Fehlerbehebung gedacht ist. Was Sie tun, ist 'clever' und 'clever' Code veranlasst die Betreuer oft zu murmeln: "Was zum ... oh, ich glaube, ich verstehe." Besser für Leute, die Ihren Code lesen, zu murmeln: "Ja, ähm, macht Sinn, ja ..."

Was ist, wenn Sie die Statusmutation von Accessor-Aufrufen getrennt haben? Ich würde mir vorstellen, dass das Lesbarkeitsproblem, mit dem Sie sich befassen, in Frage kommt, aber ich weiß nicht, wie sich das auf Ihre breitere Abstraktion auswirkt. Zumindest etwas zu beachten.

Erik Dietrich
quelle
1
+1 für den "cleveren" Kommentar. Das fängt dieses Beispiel sehr genau ein.
2
Die Aktion 'return and advance' von next () wird dem OP von der unhandlichen Java-Iterator-Schnittstelle aufgezwungen.
Kevin Cline
37

Rein vom Standpunkt des Stils aus denke ich, dass diese drei Linien:

T current = next;
next = fetcher.getNext();
return current;

... sind offensichtlicher und kürzer als ein try / finally-Block. Da Sie nicht damit rechnen, dass Ausnahmen ausgelöst werden, wird die Verwendung eines tryBlocks die Leute nur verwirren.

StriplingWarrior
quelle
2
Ein try / catch / finally-Block kann auch zusätzlichen Overhead verursachen. Ich bin mir nicht sicher, ob Javac oder der JIT-Compiler sie entfernen würden, selbst wenn Sie keine Exception auslösen.
Martijn Verburg
2
@MartijnVerburg ja, javac fügt der Ausnahmetabelle noch einen Eintrag hinzu und dupliziert den finally-Code, auch wenn der try-Block leer ist.
Daniel Lubarov
@ Daniel - danke ich war zu faul um javap auszuführen :-)
Martijn Verburg
@Martijn Verburg: AFAIK, ein Try-Catch-Finallt-Block, ist im Wesentlichen kostenlos, solange keine tatsächliche Ausnahme ausgelöst wird. Hier versucht Javac nichts und muss nichts optimieren, da JIT sich darum kümmert.
Maaartinus
@maaartinus - danke das macht Sinn - muss den OpenJDK-Quellcode nochmal lesen :-)
Martijn Verburg
6

Das würde vom Zweck des Codes im finally-Block abhängen . Das kanonische Beispiel ist das Schließen eines Streams nach dem Lesen / Schreiben, eine Art "Aufräumen", das immer durchgeführt werden muss. Das Wiederherstellen eines Objekts (in diesem Fall eines Iterators) in einen gültigen Zustand zählt IMHO ebenfalls als Bereinigung, daher sehe ich hier kein Problem. Wenn Sie OTOH verwenden return, sobald Ihr Rückgabewert gefunden wurde, und im finally-Block viel nicht verwandten Code hinzufügen, würde dies den Zweck verschleiern und alles weniger verständlich machen.

Ich sehe keine Probleme bei der Verwendung, wenn "es keine Ausnahmen gibt". Es ist sehr gebräuchlich, try...finallyohne zu verwenden, catchwenn der Code nur werfen kann RuntimeExceptionund Sie nicht vorhaben, sie zu handhaben. Manchmal ist das Endlich nur ein Schutz und Sie wissen für die Logik Ihres Programms, dass niemals eine Ausnahme geworfen wird (die klassische Bedingung "Das sollte niemals passieren").

Fallstricke: Jede Ausnahme, die im tryBlock finallyausgelöst wird, lässt den Block rennen. Das kann Sie in einen inkonsistenten Zustand versetzen. Also, wenn Ihre Rückgabeerklärung etwa so wäre:

return preProcess(next);

und dieser Code löste eine Ausnahme aus, fetchNextwürde immer noch ausgeführt. OTOH wenn du es so codiert hast:

T ret = preProcess(next);
next = fetcher.fetchNext(next);
return ret;

dann würde es nicht laufen. Ich weiß, dass Sie davon ausgehen, dass der Try-Code niemals eine Ausnahme auslösen kann, aber wie können Sie in komplexeren Fällen sicher sein? Wenn Ihr Iterator ein langlebiges Objekt wäre, würde dies auch dann fortbestehen, wenn im aktuellen Thread ein nicht behebbarer Fehler auftritt. Es wäre dann wichtig, ihn jederzeit in einem gültigen Zustand zu halten. Ansonsten ist es eigentlich egal ...

Leistung: Es wäre interessant, solchen Code zu dekompilieren, um zu sehen, wie er unter der Haube funktioniert, aber ich weiß nicht genug über die JVM, um überhaupt eine Vermutung anzustellen ... Ausnahmen sind normalerweise "außergewöhnlich", also der Code, der verarbeitet wird Sie müssen nicht für die Geschwindigkeit optimiert werden (daher der Ratschlag, niemals Ausnahmen im normalen Kontrollfluss Ihrer Programme zu verwenden), aber ich weiß es nicht finally.

mgibsonbr
quelle
Bieten Sie eigentlich keinen guten Grund, diese Art der Nutzung zu vermeidenfinally ? Ich meine, wie Sie sagen, der Code hat am Ende unerwünschte Konsequenzen. Schlimmer noch, Sie denken wahrscheinlich nicht einmal über Ausnahmen nach.
Andres F.
2
Die normale Geschwindigkeit des Steuerflusses wird durch die Konstrukte zur Ausnahmebehandlung nicht wesentlich beeinflusst. Die Leistungsstrafe wird nur gezahlt, wenn tatsächlich Ausnahmen geworfen und abgefangen werden, nicht, wenn im normalen Betrieb ein Try-Block oder ein finally-Block eingegeben wird.
Yfeldblum
1
@AndresF. Stimmt, aber das gilt auch für jeden Bereinigungscode. Angenommen, ein Verweis auf einen offenen Stream wird nulldurch einen Buggy-Code festgelegt. Der Versuch, daraus zu lesen, löst eine Ausnahme aus, der Versuch, ihn im finallyBlock zu schließen, löst eine weitere Ausnahme aus. Außerdem ist dies eine Situation, in der der Status der Berechnung keine Rolle spielt. Sie möchten den Stream schließen, unabhängig davon, ob eine Ausnahme aufgetreten ist oder nicht. Es könnte auch andere Situationen geben. Aus diesem Grund betrachte ich es eher als eine Falle für die gültigen Verwendungen als als eine Empfehlung, es niemals zu verwenden.
mgibsonbr
Es gibt auch das Problem, dass, wenn sowohl der try- als auch der finally-Block eine Ausnahme auslösen, die Ausnahme im try von niemandem gesehen wird, dies jedoch wahrscheinlich die Ursache des Problems ist.
Hans-Peter Störr
3

Die Titelangabe: "... finally-Klausel für die Ausführung von Arbeiten nach der Rückkehr ..." ist falsch. Der finally-Block wird ausgeführt, bevor die Funktion zurückkehrt. Das ist der springende Punkt der Tatsache.

Was Sie hier missbrauchen, ist die Reihenfolge der Auswertung, in der der Wert von next für die Rückgabe gespeichert wird, bevor Sie ihn ändern. Es ist keine übliche Praxis und meiner Meinung nach falsch, da Sie dadurch nicht sequentiell codieren und daher viel schwerer zu befolgen sind.

Die beabsichtigte Verwendung von finally-Blöcken ist für die Ausnahmebehandlung vorgesehen, bei der ungeachtet der ausgelösten Ausnahmen einige Konsequenzen auftreten müssen, z.

Nitsan Wakart
quelle
+1, würde ich hinzufügen, dass selbst wenn die Sprachspezifikation garantiert, dass der Wert gespeichert wird, bevor er zurückgegeben wird, er potenziell den Erwartungen des Lesers widerspricht und daher nach Möglichkeit vermieden werden sollte. Das Debuggen ist doppelt so schwer wie das Codieren ...
jmoreno
0

Ich wäre sehr vorsichtig, da (im Allgemeinen, nicht in Ihrem Beispiel) ein Teil von try eine Ausnahme auslösen kann, und wenn es weggeworfen wird, wird nichts zurückgegeben, und wenn Sie tatsächlich beabsichtigen, das auszuführen, was sich schließlich nach der Rückkehr befindet, wird es ausgeführt ausführen, wenn Sie es nicht wollten.

Und eine andere Dose Wurm ist, dass eine nützliche Aktion im Allgemeinen schließlich Ausnahmen auslösen kann, sodass Sie mit einer wirklich chaotischen Methode enden können.

Aus diesem Grund muss jemand, der es liest, über diese Probleme nachdenken, sodass es schwerer zu verstehen ist.

user470365
quelle