Wieso sollte man Null-Prüfungen nicht mit Optional-Objekten nutzen?

[Zrebna]

Hi!

Dieser SonarQube Warnhinweis hat mein Interesse erneut geweckt - siehe Bild im Anhang.

Folgendes ist in dem Code passiert:

public void foo(Optional<String> identifier) {
    if(identifier == null) {
        // some action
    }
}

Ich würde hier nie ein Optional-Objekt auf Null checken sondern so:

if(!identifier.isPresent()) {...}

// oder
if(identifier.isEmpty()) {...}

Aber ich will verstehen warum wir das im Kontext einer langfristigen Verringerung von Fehleranfälligkeit einer Software so machen und nicht wie im ersten Code-Snippet?

Mich interessiert an der Stelle nicht, dass wir Optional-Objekte nicht auf Null checken, weil wir gegen irgendwelche Verträge verstoßen, Verwirrung kreiieren oder die Grundidee, dass Optionals an Nulls "ersetzen".

Das sind alles gute Gründe, aber ich will verstehen, warum uns diese Empfehlung dabei hilft langfristig Fehler zu vermeiden?
Am besten an einem konkreten Beispiel.

Lg
Zrebna

 

[Oneixee5]

Zunächst: Optional soll man nicht in Parametern verwenden. Optional ist nur sinnvoll als Rückgabe. Damit erzwingt man die Behandlung von null, falls es keine andere sinnvolle Rückgabelmöglichkeit gibt. Optional als Parameter ist vermeidbarer Overhead. Die Verwendung von optionalen Parametern, die eine bedingte Logik innerhalb der Methoden verursachen, ist buchstäblich kontraproduktiv. Weiterhin ist die Notwendigkeit, ein Argument in ein Optional zu packen, für den Compiler suboptimal und führt zu einem unnötigen Wrapping.

 

[Oneixee5]

Ich würde hier nie ein Optional-Objekt auf Null checken sondern so:


Java:

if(!identifier.isPresent()) {...}

// oder
if(identifier.isEmpty()) {...}

So etwas würde nur in Ausnahmefällen machen. Ich würde eher eine Verzweigung verwenden wie z.B. orElseGet, orElseThrow oder ifPresentOrElse.
Diese if-else-Spagetti-Treppen-Konstrukte will man ja gerade vermeiden. Man will ja damit besseren Code mit weniger Komplexität erreichen.

 

[KonradN]

Damit erzwingt man die Behandlung von null, falls es keine andere sinnvolle Rückgabelmöglichkeit gibt.

Das ist wirklich der ganz wichtige Kernpunkt. Und das führt aus meiner Sicht zu paar Schlussfolgerungen:
a) es gibt nie eine null „Zuweisung“ zu einem Optional. (Zuweisung im erweiterten Sinn, also auch automatische Initialisierung oder Return Befehl.
b) es gibt keine Optional Fields (die würden ja automatisch mit null initialisiert)
c) Deklaration und Nutzung ist beieinander - Also Methode mit Rückgabe oder lokale Variable

Und durch diese Vorgaben können statische Codeanalysen da direkt meckern. Das hilft (neben den Code Reviews mit obligatorischer Peitsche)

Bezüglich Parameter: eher unüblich, aber auch das geht …. Gerade wenn man viel mit Optionals macht und es die Methode ggf. vereinfacht (s.u.). Ich habe es nur, wenn ich bei einem Refactoring hat an der Stelle ein Optional habe und weitere Refactorings den Code in erster Linie aufblasen / unleserlich machen (Vermutlich oft einfach nur ein Ausdruck für: Ich bin im Augenblick zu faul es richtig zu machen. In Legacy Code bricht man halt ab sonst hat man Jahre Arbeit vor sich …)

Aber ja - das sind best Practices aber sie helfen - zusammen mit dem zweiten Hinweis:

Ich würde eher eine Verzweigung verwenden wie z.B. orElseGet, orElseThrow oder ifPresentOrElse.

Das ist das A und O dabei, Code soll ja übersichtlich sein. Und das ist eben der zweite, sehr positive Punkt!

Also zusammengefasst: eigentlich war schon alles gesagt - und ich habe es nur mit etwas eigenen Worten noch einmal versucht zu erläutern.

 

[Hough]

b) es gibt keine Optional Fields (die würden ja automatisch mit null initialisiert)

Würd ich nicht ganz so eng sehen. Schließlich läßt sich ein Optional-Feld ja direkt mit = Optional.empty() initialisieren. Und das kann man durchaus gebrauchen, wenn es den Fall gibt, dass ein Wert unbekannt ist. Eine Alternative wäre das null-object-Pattern.

 

[Oneixee5]

Optional-Feld ja direkt mit = Optional.empty() initialisieren

Wozu sollte das gut sein?

 

[Hough]

Wozu sollte das gut sein?

Wenn ein Wert erst später berechnet wird und man nicht mit null-Abfragen um sich werfen möchte.

 

[Oneixee5]

Nochmal, das macht keinen Sinn:

    Optional<String> foo(String param) {
        Optional<String> ret = Optional.empty();
        ret = Optional.of(doAnything(param));
        return ret;
    }

    Optional<String> foo(String param) {
        return Optional.of(doAnything(param));
    }

 

[Hough]

Nochmal, das macht keinen Sinn:

    Optional<String> foo(String param) {
        Optional<String> ret = Optional.empty();
        ret = Optional.of(doAnything(param));
        return ret;
    }

    Optional<String> foo(String param) {
        return Optional.of(doAnything(param));
    }

Es ging um Felder in Objektklassen, nicht um lokale Variablen.

 

[Oneixee5]

Es ging um Felder in Objektklassen, nicht um lokale Variablen.

Das ist doch das selbe Prinzip. Optional ist nur ein Wrapper, den muss man nicht halten. Man kann die Instanz nicht mehr ändern, wenn sie einmal erstellt wurde.

 

[Hough]

Das ist doch das selbe Prinzip.

Wie Du meinst. Ist ja auch ne Frage persönlicher Präferenzen. Null, Null-Object-Pattern, Optionals -> drei Möglichkeiten, um anzuzeigen, dass ein Wert nicht existiert. Kommt halt vor, dass in einem Objekt ein Wert erst später berechnet wird, aber von mehr als nur 1 Methode gebraucht wird. z.B. wenn etwas berechnet und dann später in der Swing'schen paintComponent-Methode gezeichnet wird, sofern der Wert existiert.

 

[Hough]

Das ist doch das selbe Prinzip. Optional ist nur ein Wrapper, den muss man nicht halten. Man kann die Instanz nicht mehr ändern, wenn sie einmal erstellt wurde.

Jetzt noch meine Antwort auf den von Dir nachträglich hinzugefügten Teil 😉
Ich vermute, wir reden gerade aneinander vorbei. Den Wert innerhalb des Optionals kann man natürlich nicht ändern. Aber wenn das Optional ein Feld eines Objektes ist, kann das Objekt natürlich jederzeit eine neue Instanz eines Optionals erzeugen und seinem Feld zuweisen.

 

[Zrebna]

Zunächst: Optional soll man nicht in Parametern verwenden. Optional ist nur sinnvoll als Rückgabe. Damit erzwingt man die Behandlung von null, falls es keine andere sinnvolle Rückgabelmöglichkeit gibt. Optional als Parameter ist vermeidbarer Overhead. Die Verwendung von optionalen Parametern, die eine bedingte Logik innerhalb der Methoden verursachen, ist buchstäblich kontraproduktiv. Weiterhin ist die Notwendigkeit, ein Argument in ein Optional zu packen, für den Compiler suboptimal und führt zu einem unnötigen Wrapping.

Im Idealfall "erzwingt" man sie, weil die Verwendung eines Optionals ja quasi dem Entwickler sagen soll, dass etwas "leer" sein kann bzw. keinen Wert enthalten könnte, worauf der Entwickler automatisch prüft, ob das Optional-Objekt leer ist oder einen Wert hat. Außer den Entwickler "weigert" sich, wie hier in der Abbildung unten.


Genau hier finde ich das Warning 100% relevant im Sinne einer Fehlervorbeugung, weil durch die fehlende Überprüfung an dieser Stelle tatsächlich ein Fehler in Form einer NoSuchElelementException auftreten kann. Richtig?

 

[Oneixee5]

Der Fall ist doch klar. Es wird an die Methode ein Optional übergeben. Dieser Umstand selbst macht deutlich, dass hier etwas zu beachten ist. Der Vorschlag von Sonar ist so semi. Sonar kennt aber auch nicht den Sinn deines Programm. Also macht Sonar sichere Vorschläge.

So etwas würde ein erfahrener Entwickler aber nicht machen:

if(!identifier.isPresent()) {...}

// oder
if(identifier.isEmpty()) {...}

Wenn man funktional programmiert, dann sollte man das auch richtig machen und sich darauf einlassen. Je nach Context kann man das verschieden behandeln. Hier eine Möglichkeit von vielen:

    void foo(Optional<Long> oId) {
        oId.ifPresentOrElse(this::doWithId, this::doWithoutId);      
    }

    void doWithId(Long id) {
        // ->
    }
  
    void doWithoutId() {
        // ->
    }

Hier verzweigt man im Programm, je nachdem auf welchen Zustand man trifft. Es wird keine Methode mit X-Zeilen Code für den if- oder else-Zweig erstellt. Es erfolgt eine einfache Entscheidung: wenn das mache das, ansonsten das. So kann man auch super mit Vererbung agieren, denn die Logik ist immer klar und wird immer verstanden.

Für Java-Programmierer ist das Vorgehen erstmal etwas schwer oder wie ein Fremdkörper. Man muss sich darauf einlassen um den Flow zu verstehen. Das liegt einfach daran, das Java auch schon eine alte Sprache ist und keine breaking changes will. Code, der in älteren Versionen gültig war, soll auch zukünftig gültig sein. (Das ist keine Kritik) Also sickert diese Art, etwas durchzuführen langsamer in Java ein, als in junge dynamische Sprachen, wie z.B. Golang oder Rust. Dort sind quasi alle Api's auf diese Art gestaltet. Ein if-else findet man dort, bei guten Leuten, so gut wie nicht. Die Arbeitsweise ist stabil und fehlerresistent - und darum geht es am Ende. Jede Möglichkeit muss behandelt werden - den Milliarden-Dollar-Fehler kann man damit schon mit dem Compiler vermeiden.
Ein anderes Problem sind die Java-Kurse oder Lehrer. Es wird auf Neuerungen in der Sprache oft sehr wenig Wert gelegt. Ich habe erst kürzlich einen Kurs gesehen, welcher mit Java 1.6 aufgebaut war. Der Kursleiter hielt den Kurs für großartig. Was aber dann im Extremfall passiert: der Kunde lässt unseren Code von seinem Experten prüfen - dieser kann aber unseren Code gar nicht lesen/interpretieren ...