Warum ist Optional.get () ohne Aufruf von isPresent () schlecht, aber nicht iterator.next ()?

23

Wenn ich die neue Java8-Streams-API verwende, um ein bestimmtes Element in einer Sammlung zu finden, schreibe ich folgenden Code:

    String theFirstString = myCollection.stream()
            .findFirst()
            .get();

Hier warnt IntelliJ, dass get () aufgerufen wird, ohne vorher isPresent () zu überprüfen.

Dieser Code:

    String theFirstString = myCollection.iterator().next();

... gibt keine Warnung aus.

Gibt es einen tiefgreifenden Unterschied zwischen den beiden Techniken, der den Streaming-Ansatz in gewisser Weise "gefährlicher" macht, sodass es entscheidend ist, get () nie aufzurufen, ohne isPresent () zuerst aufzurufen? Beim googeln finde ich Artikel darüber, wie nachlässig Programmierer mit Optional <> sind, und gehe davon aus, dass sie get () aufrufen können, wann immer dies der Fall ist.

Ich möchte, dass eine Ausnahme so nah wie möglich an der Stelle ausgelöst wird, an der mein Code fehlerhaft ist. In diesem Fall würde ich get () aufrufen, wenn kein Element gefunden werden kann. Ich möchte nicht unnötige Aufsätze schreiben müssen wie:

    Optional<String> firstStream = myCollection.stream()
            .findFirst();
    if (!firstStream.isPresent()) {
        throw new IllegalStateException("There is no first string even though I totally expected there to be! <sarcasm>This exception is much more useful than NoSuchElementException</sarcasm>");
    }
    String theFirstString = firstStream.get();

... es sei denn, es besteht die Gefahr, Ausnahmen von get () auszulösen, die mir nicht bekannt sind?


Nachdem ich die Antwort von Karl Bielefeldt und einen Kommentar von Hulk gelesen hatte, stellte ich fest, dass mein Ausnahmecode oben etwas ungeschickt war. Hier ist etwas Besseres:

    String errorString = "There is no first string even though I totally expected there to be! <sarcasm>This exception is much more useful than NoSuchElementException</sarcasm>";
    String firstString = myCollection.stream()
            .findFirst()
            .orElseThrow(() -> new IllegalStateException(errorString));

Dies sieht nützlicher aus und wird wahrscheinlich an vielen Stellen natürlich vorkommen. Ich habe immer noch das Gefühl, dass ich in der Lage sein möchte, ein Element aus einer Liste auszuwählen, ohne mich mit all dem befassen zu müssen, aber das könnte nur sein, dass ich mich an diese Art von Konstrukten gewöhnen muss.

Der Fernseher ist Frank
quelle
7
Wenn Sie orElseThrow verwenden, könnte der Teil, der die Ausnahmebedingung Ihres letzten Beispiels auslöst, wesentlich prägnanter sein - und allen Lesern wäre klar, dass Sie beabsichtigen, die Ausnahmebedingung auszulösen .
Hulk

Antworten:

12

Bedenken Sie zunächst, dass die Prüfung komplex und wahrscheinlich relativ zeitaufwendig ist. Es ist eine statische Analyse erforderlich, und zwischen den beiden Aufrufen befindet sich möglicherweise ziemlich viel Code.

Zweitens ist die idiomatische Verwendung der beiden Konstrukte sehr unterschiedlich. Ich programmiere Vollzeit in Scala, das Optionsviel häufiger verwendet als Java. Ich kann mich nicht an eine einzelne Instanz erinnern, bei der ich a .get()auf a verwenden musste Option. Sie sollten fast immer die Optionalvon Ihrer Funktion zurückgeben oder orElse()stattdessen verwenden. Dies sind viel bessere Möglichkeiten, fehlende Werte zu behandeln, als Ausnahmen auszulösen.

Da .get()diese Funktion im normalen Gebrauch nur selten aufgerufen werden sollte, ist es für eine IDE sinnvoll, eine komplexe Prüfung zu starten, wenn eine angezeigt wird, um festzustellen, .get()ob sie ordnungsgemäß verwendet wurde. Im Gegensatz dazu iterator.next()ist die ordnungsgemäße und allgegenwärtige Verwendung eines Iterators.

Mit anderen Worten, es geht nicht darum, dass das eine schlecht ist und das andere nicht, sondern darum, dass das eine ein allgemeiner Fall ist, der für seine Funktionsweise von grundlegender Bedeutung ist und bei Entwicklertests höchstwahrscheinlich eine Ausnahme auslöst, und das andere wird nur selten verwendet Fall, der möglicherweise nicht ausreichend ausgeübt wird, um beim Entwicklertest eine Ausnahme auszulösen. Dies sind die Fälle, vor denen IDEs Sie warnen sollen.

Karl Bielefeldt
quelle
Könnte sein, dass ich mehr über dieses optionale Geschäft erfahren muss - ich habe Java8 meistens als eine gute Möglichkeit gesehen, Listen von Objekten zu mappen, die wir häufig in unserer Webanwendung verwenden. Sie sollen also überall optionale <> s senden? Das wird gewöhnungsbedürftig sein, aber das ist ein weiterer SE-Beitrag! :)
Fernseher Frank
@TV's Frank Es ist weniger so, dass sie überall sein sollen, als vielmehr, dass Sie damit Funktionen schreiben können, die einen Wert des enthaltenen Typs transformieren, und Sie verketten sie mit mapoder flatMap. OptionalDies ist meistens eine gute Möglichkeit, um zu vermeiden, dass alles mit nullSchecks belegt wird.
Morgen
15

Die optionale Klasse soll verwendet werden, wenn nicht bekannt ist, ob das darin enthaltene Element vorhanden ist oder nicht. Die Warnung wird angezeigt, um Programmierer darüber zu informieren, dass ein zusätzlicher möglicher Codepfad vorhanden ist, den sie möglicherweise ordnungsgemäß verarbeiten können. Die Idee ist zu vermeiden, dass Ausnahmen überhaupt geworfen werden. Der von Ihnen präsentierte Anwendungsfall ist nicht für diesen Zweck vorgesehen. Daher ist die Warnung für Sie irrelevant. Wenn Sie tatsächlich möchten, dass eine Ausnahme ausgelöst wird, deaktivieren Sie sie (allerdings nur für diese Zeile, nicht global) Wenn Sie die Entscheidung treffen, ob der nicht vorliegende Fall instanziell behandelt werden soll, werden Sie durch die Warnung daran erinnert, dies zu tun.

Möglicherweise sollte eine ähnliche Warnung für Iteratoren generiert werden. Es wurde jedoch einfach noch nie getan, und das Hinzufügen einer neuen Warnung würde wahrscheinlich zu viele Warnungen in vorhandenen Projekten zur Folge haben, um eine gute Idee zu sein.

Beachten Sie auch, dass das Auslösen einer eigenen Ausnahme nützlicher sein kann als nur eine Standardausnahme, da es eine Beschreibung des erwarteten Elements enthält, das beim späteren Debuggen jedoch nicht sehr hilfreich sein kann.

Jules
quelle
6

Ich bin damit einverstanden, dass es keinen inhärenten Unterschied gibt. Ich möchte jedoch auf einen größeren Mangel hinweisen.

In beiden Fällen haben Sie einen Typ, der eine Methode bereitstellt, deren Funktion nicht garantiert ist, und davor eine andere Methode aufrufen soll. Ob Sie von Ihrer IDE / Ihrem Compiler / etc vor dem einen oder anderen Fall gewarnt werden, hängt davon ab, ob dieser Fall unterstützt wird und / oder ob Sie ihn dafür konfiguriert haben.

Davon abgesehen sind beide Codeteile Code-Gerüche. Diese Ausdrücke weisen auf zugrunde liegende Konstruktionsfehler hin und führen zu sprödem Code.

Sie haben natürlich Recht, wenn Sie schnell scheitern wollen, aber die Lösung, zumindest für mich, kann nicht darin bestehen, die Schultern abzuziehen und Ihre Hände in die Luft zu werfen (oder eine Ausnahme auf den Stapel zu werfen).

Möglicherweise ist Ihre Sammlung vorerst nicht leer, aber Sie können sich nur auf den Typ verlassen: Collection<T>- Und das garantiert Ihnen nicht, dass Sie leer sind . Auch wenn Sie jetzt wissen, dass es nicht leer ist, kann eine geänderte Anforderung dazu führen, dass der Code im Voraus geändert wird und Ihr Code plötzlich von einer leeren Sammlung getroffen wird.

Jetzt können Sie zurückschieben und anderen mitteilen, dass Sie eine nicht leere Liste erhalten sollten. Sie können froh sein, dass Sie diesen "Fehler" schnell finden. Trotzdem haben Sie eine kaputte Software und müssen Ihren Code ändern.

Vergleichen Sie dies mit einem pragmatischeren Ansatz: Da Sie eine Sammlung erhalten, die möglicherweise leer ist, müssen Sie sich mit diesem Fall befassen, indem Sie Optional # map, #orElse oder eine andere dieser Methoden außer #get verwenden.

Der Compiler erledigt so viel Arbeit wie möglich für Sie, sodass Sie sich so weit wie möglich auf den statischen Vertrag verlassen können Collection<T>. Wenn Ihr Code niemals gegen diesen Vertrag verstößt (z. B. über eine dieser beiden stinkenden Anrufketten), ist Ihr Code für sich ändernde Umgebungsbedingungen weniger anfällig.

Im obigen Szenario hat eine Änderung, die zu einer leeren Eingabeauflistung führt, keinerlei Auswirkungen auf Ihren Code. Es ist nur eine weitere gültige Eingabe und wird daher weiterhin korrekt verarbeitet.

Die Kosten dafür sind, dass Sie Zeit in bessere Designs investieren müssen, anstatt a) spröden Code zu riskieren oder b) Dinge unnötig zu komplizieren, indem Sie Ausnahmen herumwerfen.

Abschließend gilt: Da nicht alle IDEs / Compiler ohne vorherige Überprüfung vor iterator (). Next () oder optional.get () warnen, wird dieser Code in der Regel in Überprüfungen markiert. Für das, was es wert ist, würde ich nicht zulassen, dass ein solcher Code eine Überprüfung besteht, und stattdessen eine saubere Lösung fordern.

Frank
quelle
Ich schätze, ich kann ein bisschen dem Geruch von Code zustimmen - ich habe das obige Beispiel gemacht, um einen kurzen Beitrag zu verfassen. Ein weiterer sinnvollerer Fall könnte darin bestehen, ein obligatorisches Element aus einer Liste zu entfernen, was in einer IMO-Anwendung nicht ungewöhnlich ist.
Fernseher Frank
Spot on. "Wählen Sie das Element mit der Eigenschaft p aus der Liste" -> list.stream (). Filter (p) .findFirst () .. und noch einmal müssen wir die Frage stellen "Was ist, wenn es dort nicht vorhanden ist?" .. täglich Brot und Butter. Wenn es Mandatoray und "Just There" ist - warum hast du es überhaupt in einer Menge anderer Sachen versteckt?
Frank
4
Weil es vielleicht eine Liste ist, die aus einer Datenbank geladen wird. Vielleicht ist die Liste eine statistische Sammlung, die in einer anderen Funktion gesammelt wurde: Wir wissen, dass wir Objekte A, B und C haben. Ich möchte die Menge von allem für B herausfinden. Viele gültige (und in meiner Anwendung häufig vorkommende) Fälle.
Fernseher Frank
Ich weiß nichts über Ihre Datenbank, aber meine Datenbank garantiert nicht mindestens ein Ergebnis pro Abfrage. Gleiches gilt für statistische Sammlungen - wer sagt, dass diese immer leer bleiben? Ich bin damit einverstanden, dass es einfach zu begründen ist, dass es dieses oder jenes Element geben sollte, aber ich bevorzuge die richtige statische Typisierung gegenüber der Dokumentation oder, schlimmer noch, einige Erkenntnisse aus einer Diskussion.
Frank