Skip to content

return null;

In seinem Buch "Clean Code" empfiehlt Robert C. Martin, nicht explizit "null" zurück zu geben. Weil es Arbeit erzeugt und den Code mit null-checks unleserlich macht.
Ich bin aber der Meinung, dass man das nicht verallgemeinern kann und sollte. An folgenden Beispiel will ich das einmal deutlich machen.

These:
Wenn wir einen DAO haben, mit einer Methode "findItemById(id)" dann soll "null" zurück geben werden, wenn kein Item mit dieser Id existiert!

Begründung:
Zum einen bekommen wir mit dem Wert "null" genau das zurück, was wir erwarten würden. Denn wir müssen davon ausgehen, dass in dem Repository oder der DB die wir da abfragen kein Eintrag mit dieser Id existiert.
Es wäre idiotisch zu erwartet, dass die DB einen Eintrag für jede Id hat, die wir uns ausdenken können - es zwingt uns schließlich niemand, der Methode nur Ids zu übergeben, für die ein Eintrag existiert!
Die Methode in dem DAO gibt uns also genau das zurück, was in der DB steht: kein Eintrag! Und "null" ist IMO die beste Repräsentation von "kein Eintrag".

Ich habe schon Implementationen gesehen, wo im Fall von "kein Eintrag" eine UnknownEntryException geschmissen wird. Aber das ist ja nicht richtig.
Eine Exception - also eine Ausnahme - impliziert ja, dass ein Fehler aufgetreten ist, der so nicht zu erwarten war. Aber das stimmt ja nicht. Wie bereits erläutert mussten wir ja zwingend davon ausgehen, dass so ein Fall eintreten könnte. Und es ist auch kein Fehler! Einen Eintrag mit einer beliebigen Id in einer DB nicht zu finden ist kein Fehler sondern normal.

Uncle Bob empfiehlt in seinem Buch "special case"-Objects zu verwenden. Aber in dem Fall würde das ja nur noch mehr Verwirrung stiften. Denn es wäre noch schwerer zu erkennen ob er den Eintrag in der DB gefunden hat, und dieser einfach nur leer ist, oder ob der Eintrag nicht existiert. (Special-Case Objekte sind nur dann sinnvoll, wenn ich nicht explizit diese Unterscheidung machen muss, weil sie ggfs. nicht relevant ist)

Prüfen muss ich es in dieser Konstellation nämlich in jedem Fall!
Das heißt, egal was ich mache um das Zurückgeben von "null" zu vermeiden - die Arbeit, den Fall "kein Eintrag gefunden" zu checken, kann ich mir dadurch nicht ersparen!
Wenn ich es doch machen würde, hieße das: ich frage explizit etwas an, schere mich aber einen Dreck um das Ergebnis - das wäre hochgradig Verantwortungslos. (Zumindest in den allermeisten Fällen)

Da wir nun also festgestellt haben, dass wir eine Überprüfung machen müssen, schauen wir doch mal, wie diese am Einfachsten realisiert werden kann (KISS-Prinzip).

Fall 1 - "return null":
ResultObject result = dao.findItemById(42);
if(result == null) {
    return;
}
Ich denke dazu brauch ich nicht viel schreiben: kurz, auf den Punkt, sofort verständlich.


Fall 2 - "throw Exception":
ResultObject result;
try {
    result = dao.findItemById(42);
} catch(UnknownEntryException e) {
    return;
}
Doppelt so viele Zeilen wie in Fall 1.
Es macht den Code außerdem unleserlicher und kann zu Verwirrung führen, da nicht auf den ersten Blick erkenntlich ist, ob hier tatsächlich ein Fehlerfall behandelt wird.
Dass es keine Option ist, das try-catch-statement einfach weg zu lassen, muss ich hoffentlich nicht extra erwähnen (das hab ich oben schon genug getan).


Fall 3 - "Special case object 1"
ResultObject result = dao.findItemById(42);
if(result.getId() == 0) {
    return;
}
Wie ein Special-Case-Object gestalltet wird, ist natürlich nicht fest vorgegeben. Hier also der Fall, dass es einfach ein normales ResultObject mit leeren Feldern ist.
Und wir sehen direkt die Probleme die damit einher gehen! Zum einen ist nicht klar, ob "0" nicht eventuell eine gültige Id sein könnte.
In diesem Fall setzen wir auch voraus, dass die Id, mit der wir anfragen auch Teil des Ergebnis ist. Wenn sie das nicht wäre, könnten wir nicht mehr unterscheiden, ob der Eintrag gefunden wurde und einfach nur leer ist oder ob er nicht gefunden wurde.
Das nächste Problem damit, erläutere ich gleich in Fall 4.


Fall 4 - "Special case object 2"
ResultObject result = dao.findItemById(42);
if(result.isNoEntry()) {
    return;
}
In diesem Fall haben wir dass Problem, dass wir unser Ergebnis-Objekt um eine Methode oder ein Feld erweitern müssen, um diesen Fall explizit abzufragen.
(bzw. wird es sich wohl eher um eine DataStructure handeln, aber in Java gibt es da keine so explizite Unterscheidung, also nennen wir es der Einfachheit halber ein Objekt)
Das führt direkt zu einem weiteren Problem: wie benennt man so etwas? "isEmpty()" wäre z.B. schlecht, weil jemand der den Code ließt, davon ausgehen könnte, es handelt sich um ein Objekt dass von Collection erbt.
Aber auch bei "isNoEntry()" stellt sich die Frage, ob es sich dabei um einen Wert handelt, der so im Eintrag in der DB steht.
Und was das schon in Fall 3 angeteaserte Problem angeht: bei diesem Code wird die verwendete IDE vermutlich anmeckern, dass "result" null sein könnte. Aber selbst wenn nicht: normalerweise arbeitet man ja nicht alleine. Und sobald ein Kollege den Code anfasst, wird ihm (oder ihr) wahrscheinlich auffallen, dass dort ein null-check fehlt! Und dann sieht der Code ganz schnell so aus:

ResultObject result = dao.findItemById(42);
if(result == null || result.isNoEntry()) {
    return;
}
Was fällt uns auf? Genau: der Code sieht so aus wie in Fall 1 - nur mit einem zusätzlichen Aufruf von "isNoEntry"!
Wäre es nicht einfacher, diesen zusätzlichen Aufruf einfach weg zu lassen? Dann könnten wir auch darauf verzichten, das ResultObject zu erweitern.


Schlussfolgerung:
Wenn wir in einem Repository ein bestimmtes Item abfragen, dann ist es sinnvoll "null" zurück zu geben, wenn dieses Item nicht existiert!


Zusätzliche Anmerkung:
Und dass mir jetzt keiner mit dem Vorschlag kommt, doch stattdessen eine Liste mit nur einem Eintrag zurück zu geben!
Nicht nur, dass wir dem Aufrufer (also dem Nutzer des DAO) damit zusätzliche und unnötige Arbeit aufhalsen, weil dieser auch noch den unwahrscheinlichen Fall abfragen muss, falls mehr als ein Item in der Liste ist.
Es ist unnötige Ressourcenverschwendung, dass Item noch mal in eine Liste zu packen, die sonst keinen Zweck erfüllt.
Außerdem wird das API des DAO dadurch kontra-intuitiv und umständlicher zu benutzen (um es mal vorsichtig zu formulieren).

Nachtrag:
Eine zusätzliche Methode im DAO "existsItemWithId(id)" die ein boolean zurück gibt, halte ich im Hinblick auf Performance ebenfalls für untauglich. Die Datenbank ist ohnehin schon das Bottleneck der gesammten Applikation - da muss man nicht noch eine zweite Abfrage absetzen, wenn eine auch reichen würde.

Trackbacks

Keine Trackbacks