Optional-Idiome

Ich sammle Material für kürzere Vorträge für meine Kollegen, und will mit etwas einfachen anfangen - Optional. Heute bin ich beim Code-Review über folgenden Code gestolpert:

public Optional<Plonk> createPlonk(Foo foo) {
    if (foo instanceof Bar) {
        return Optional.of(makePlonkFromBar((Bar) foo));
    }
    return Optional.empty();
} 

Mein Vorschlag:

public Optional<Plonk> createPlonk(Foo foo) {
    return Optional.of(foo)
        .filter(Bar.class::isInstance)
        .map(Bar.class::cast)
        .map(Plonk::makePlonkFromBar);
} 

Ich denke, das sieht deutlich besser aus.

Kennt ihr ähnliche Idiome im Zusammenhang mit Optional?

Hi, mal eine Frage, wenn ich gerade den Code lese…habe ich den Eindruck, dass der von Dir gemachte Vorschlag nicht „Null“ safe ist…im Gegensatz zur Lösung mit instanceof ?

Wenn mich gerade mein Gedächtnis nicht verläßt, dann darf doch Parameter bei Optional.of(..) nicht null sein? Somit müsste das Optional.of(..) durch ein Optional.ofNullable(..) ersetzt werden?..Oder habe ich noch was übersehen?

Gruß
Karl Heinz

Ja, das ist korrekt, wenn foo auch null sein kann. Aber das versuchen wir (wenigstens in neuem Code) möglichst zu vermeiden.

Abgesehen davon, dass ich instanceof meistens als Zeichen sehe, dass etwas nicht so richtig durchdacht ist (was man aber durch die Platzhalternamen hier nicht mehr erkennt), und man Optional auch überbeanspruchen kann: Was ist das Ziel? Soll wirklich existierender Code ersetzt werden?

(Ob die fluent-Variante nun schöner ist, darüber kann man streiten (ich finde nicht)).

1 Like

das versuche ich auch schon seit Jahren - sitze gerade wieder an einem Problem, wo ich nicht weis wo dieser dämliche null-Pointer her kommt :crazy_face: - der Code existiert&läuft nun schon seit Jahren

ansonsten finde ich auch die erste Variante schöner

Okay, das klare Votum für die erste Variante überrascht mich jetzt.

Ist vielleicht auch eine Sache der Gewohnheit. Für mich passiert in Variante eins „alles auf einmal“, während in der Fluent-Variante Schritt für Schritt vorgegangen wird: Behalte nur Objekte mit dem richtigen Typ, caste zu diesem Typ, mappe zum endgültigen Ergebnis. Einfacher kann man den Ablauf kaum machen.

Aber so eine Umfrage war eigentlich nicht Ziel des Threads, ich wollte andere Tipps und Tricks mit Optionals sammeln. Die Varianten oben waren eigentlich nur als Beispiel dafür gedacht, was ich suche.

A: Ich suche Punkte im dreidimensionalen Raum. Zum Beispiel: (1.0, 0.5, 0.2).
B: Ein weiteres Beispiel wäre (3.5, 1.7776, 1242342.5674)
A: Nein nein, die sollten schon im Einheitswürfel liegen!

Ich denke, es kommt stark darauf an, welche „Muster“ in der Codebasis vorkommen, und wie man die „idiomatisch“ oder „schön“ durch Optional ersetzen kann (oder will? oder sollte?).

Ich hatte irgendwann mal ein paar Optionals-Methoden erstellt, die mit „dem“ Optional nichts zu tun haben, aber ggf. auch dadurch ersetzt werden könnten. Sowas wie

Map<K, V> map = Optional.ofNullable(input).orElse(Collections.emptyMap());

kann man schreiben, aber wenn das 100 mal vorkommt, ist

Map<K, V> map = Optionals.of(input);

eben einfacher…

Analog dazu könnte man, wenn es sehr oft vorkommt, bei

Optional.of(foo)
        .filter(Bar.class::isInstance)
        .map(Bar.class::cast)
        .map(...);

das mehrfache Auftreten von Bar.class störend finden - nicht zuletzt auch im Sinne von „finde den Fehler“:

Optional.of(foo)
        .filter(Bar.class::isInstance)
        .map(Baz.class::cast)
        .map(...);

und sich sowas wie

Optionals.ifType(foo, Bar.class) // Liefert ein (hart getyptes) Optional<Bar>
    .map(...)

schustern.


Der Grund, warum dein Vorschlag auf Skepsis stößt, ist aber (vermutlich allgemein, und speziell) bei mir: Ich finde man sollte Optional NICHT als „drop-in-replacement für if-Abfragen“ einsetzen.

Brian Goetz hat zu Optional gesagt:

Our intention was to provide a limited mechanism for library method return types where there needed to be a clear way to represent „no result“, and using null for such was overwhelmingly likely to cause errors.

Das könnte in deinem Beispiel der Fall sein. Aber da das Beispiel durch die Platzhalternamen nicht mehr so viel aussagt drängt sich die Frage auf, die ich immer versuche, mir (bewußt naiv) zu stellen: Wer soll das wann und wo und wie verwenden?

Wenn dein skizziertes Beispiel so verwendet wird

Foo foo = obtainFoo();
Optional<Plonk> optionalPlonk = createPlonk(foo);
if (optionalPlonk.isPresent()) {
    Plonk plonk = optionalPlonk.get();
    process(plonk);
}

dann ist offenbar irgendwo irgendwas ganz, ganz Sche!ße gelaufen. D.h. wenn die ganze createPlonk-Methode und die Verantwortung für die darin (egal wie!) mit dem Optional umgesetzte Prüfung auch dem Aufrufer überlassen könnte oder sollte, stellen sich manche Fragen ja gar nicht…


Ansonsten: Allgemeine „Idiome“ wären vielleicht Sachen wie „Wie verwandle ich eine Map<K, Optional<V>> in eine Map<K, V>?“. Aber das würde ich eher als „Snippets, wie man etwas (geschickt) macht“ bezeichnen…

Wenn dein skizziertes Beispiel so verwendet wird

Foo foo = obtainFoo();
Optional<Plonk> optionalPlonk = createPlonk(foo);
if (optionalPlonk.isPresent()) {
    Plonk plonk = optionalPlonk.get();
    process(plonk);
}

dann ist offenbar irgendwo irgendwas ganz, ganz Sche!ße gelaufen.

Der Vorteil der Optionals liegt gerade darin, dass man stattdessen folgendes schreiben kann:

createPlonk(obtainFoo()).ifPresent(this::process);

Wann immer eine Methode null zurückgeben kann, nutze ich inzwischen Optionals. Genau dafür sind sie gedacht.
Und ich nutze sie mitunter auch um if-Statements zu ersetzen. Nämlich immer dann, wenn der Quelltext dadurch insgesamt kürzer und übersichtlicher wird, was meistens dann der Fall ist, wenn man dadurch keine lokale Variablen deklarieren muss.
@Landei Deinen Vorschlag finde ich persönlich besser als die zweite Variante, der Vorteil von Fluent-Programming ist hier aber nicht zwingend. Man kann auch die erste Variante nutzen.
Hier noch ein Beispiel für die Nutzung von Optionals:

public static String fileExtension(String fileName) {
    return Optional.of(fileName.lastIndexOf(".")).filter(i-> i >= 0)
            .map(fileName::substring).orElse("");
}
1 Like

Man kann da sicher unterschiedliche Ansichten (aka Stile) haben. Und ich habe die Beobachtung gemacht, dass wenn ein neues Sprachfeature eingeführt wird, manche Leute dazu neigen, dieses Sprachfeature als Universalhammer für alle Nägel einzusetzen. Das habe ich beobachtet bei Generics, Enums, Streams… und Optional *räusper*.

Nochmal das Zitat von oben:

Für mich bezieht sich das auf Fälle wie den, von Map#get:

V value = map.get(key);
if (value == null) {
    // Either there was no entry for 'key', or the 'key' was mapped to 'null'
}

Oder konkreter: Ein „a limited mechanism for library method return types“ klingt für mich nicht nach etwas, was konsequent ganz normale if-Abfragen ersetzen sollte (auch wenn das so schön neu und gerade en vogue ist…).