Was ist der Sinn von Guava checkNotNull

70

Ich bin ziemlich neu in Guava (seien wir ehrlich, ich bin nicht "ziemlich neu", ich bin ein absoluter Neuling in diesem Bereich) und habe mich daher entschlossen, einige Dokumentationen durchzugehen und war beim Lesen ziemlich erstaunt:

com.google.common.base.Preconditions.checkNotNull(...)

Ich verstehe den Sinn dieser Methode nicht. Dies bedeutet, dass anstatt zu tun:

myObject.getAnything();

(was dazu führen kann, dass a, NullPointerExceptionwenn myObject null ist)

Ich sollte verwenden

checkNotNull(myObject).getAnything();

was wird ein Wurf , NullPointerExceptionwenn myObjectnull und Rückkehr ist , myObjectwenn es nicht null ist.

Ich bin verwirrt und dies könnte die dümmste Frage sein, aber ...

Was ist der Sinn davon? Diese beiden Zeilen machen genau das Gleiche wie für die Ergebnisse in allen Situationen, die mir einfallen.

Ich denke nicht einmal, dass Letzteres besser lesbar ist.

Also muss mir etwas fehlen. Was ist es?

Ar3s
quelle
1
Ich kaufe mir nicht wirklich die Idee, checkNotNull an jedem Ort hinzuzufügen. Meiner Meinung nach ist es vorzuziehen, wenn Sie Ihre Eingabe explizit überprüfen, anstatt sich zu sehr auf den syntaktischen Zucker zu verlassen, der zur Verschleierung Ihres Codes beiträgt. Ein schneller Fehler ist in Ordnung, aber checkNotNull gibt keine weiteren Informationen zur Fehlerbehebung an -> Sie können einfach Ihre eigene Ausnahme für ein falsches Eingabeformat machen und eine nette Nachricht eingeben, in der die spezifische Situation erläutert wird
Hoàng Long
Ich stimme vollkommen zu, dass dies lächerlich ist. Nach Null zu suchen und dann selbst eine NPE zu werfen, ist meiner bescheidenen Meinung nach total verrückt. Wirf mindestens eine ValidationException oder IllegalArgumentException aus.
Lawrence
Erwägen Sie eine bessere Diagnose mit checkNotNull(reference, errorMessageTemplate, errorMessageArgs): guava.dev/releases/23.0/api/docs/com/google/common/base/…
Vadzim

Antworten:

105

Die Idee ist, schnell zu scheitern. Betrachten Sie zum Beispiel diese dumme Klasse:

public class Foo {
    private final String s;

    public Foo(String s) {
        this.s = s;
    }

    public int getStringLength() {
        return s.length();
    }
}

Angenommen, Sie möchten keine Nullwerte für zulassen s. (oder getStringLengthwirft eine NPE). Wenn die Klasse so ist wie sie ist, nullist es zu spät - es ist sehr schwer herauszufinden, wer sie dort abgelegt hat. Der Täter könnte durchaus in einer völlig anderen Klasse sein, und diese FooInstanz könnte vor langer Zeit konstruiert worden sein. Jetzt müssen Sie Ihre Codebasis durchkämmen, um herauszufinden, wer dort möglicherweise einen nullWert hätte setzen können.

Stellen Sie sich stattdessen diesen Konstruktor vor:

public Foo(String s) {
    this.s = checkNotNull(s);
}

Wenn jemand dort einen Eintrag einfügt null, werden Sie es sofort herausfinden - und der Stack-Trace zeigt Sie genau auf den fehlgeschlagenen Aufruf.


Ein anderes Mal kann dies nützlich sein, wenn Sie die Argumente überprüfen möchten, bevor Sie Aktionen ausführen, mit denen der Status geändert werden kann. Betrachten Sie beispielsweise diese Klasse, die den Durchschnitt aller erhaltenen Zeichenfolgenlängen berechnet:

public class StringLengthAverager {
    private int stringsSeen;
    private int totalLengthSeen;

    public void accept(String s) {
        stringsSeen++;
        totalLengthSeen += s.length();
    }

    public double getAverageLength() {
        return ((double)totalLengthSeen) / stringsSeen;
    }
}

Beim Aufrufen accept(null)wird eine NPE ausgelöst - jedoch nicht zuvor stringsSeen. Dies ist möglicherweise nicht das, was Sie wollen. Als Benutzer der Klasse kann ich erwarten, dass der Status unverändert bleibt, wenn keine Nullen akzeptiert werden, wenn Sie eine Null übergeben (mit anderen Worten: Der Aufruf sollte fehlschlagen, das Objekt jedoch nicht ungültig werden). In diesem Beispiel können Sie das Problem natürlich auch beheben, indem Sie es s.length()vor dem Inkrementieren abrufen stringsSeen. Sie können jedoch sehen, dass es für eine längere und aufwändigere Methode hilfreich sein kann, zunächst zu überprüfen, ob alle Ihre Argumente gültig sind, und erst dann den Status zu ändern:

    public void accept(String s) {
        checkNotNull(s); // that is, s != null is a precondition of the method

        stringsSeen++;
        totalLengthSeen += s.length();
    }
yshavit
quelle
1
Sollte es nicht mehr sein this.s = Preconditions.checkNotNull (s);
Ar3s
aber ja, ich habe nicht über den Aufbewahrungskoffer nachgedacht.
Ar3s
13
Das typische Muster ist this.s = checkNotNull(s);ein statischer Import.
ColinD
1
Persönlich denke ich, dass diese Art der Fehlerberichterstattung vermieden werden sollte. Was auch immer Sie tun, eine NullPointerException hat buchstäblich wenig bis gar keine Bedeutung. Ich bin stark dagegen, in JEDER Funktion null zu übergeben / zurückzugeben (es sei denn, Sie arbeiten mit einer Legacy-API). Daher sollte jeder Versuch, dies zu tun, eine WrongApiUsage-Ausnahme ins Gesicht sehen.
Hoàng Long
3
@ HoàngLong Ich wünschte auch, das checkNotNullwürde eine IllegalArgumentException anstelle von NPE auslösen. Für mich bedeutet eine NPE, dass jemand versucht hat, einen Nullzeiger zu dereferenzieren - nicht, dass jemand die Tatsache erkannt hat, dass er null war, bevor er versucht hat, ihn zu dereferenzieren. Vor diesem Hintergrund denke ich auch, dass "IllegalArgumentException: foo was null" in keiner praktischen Hinsicht hilfreicher ist als "NullPointerException: foo". Also, während ich mir irgendwie wünsche, dass die Guaven-Leute IllegalArgumentException ausgewählt haben, ist die Bequemlichkeit und Standardisierung der Außerkraftsetzung checkNotNullzumindest für mich nicht der Fall .
Yshavit
10

myObject.getAnything(); (Dies kann eine NullPointerException verursachen, wenn myObject null ist.)

Nein ... es wird immer NPE werfen myObject == null. In Java gibt es keine Möglichkeit, eine Methode mit nullEmpfänger aufzurufen (eine theoretische Ausnahme sind statische Methoden, aber sie können und sollten immer ohne Objekt aufgerufen werden).


Ich sollte verwenden checkNotNull(myObject).getAnything();

Nein, solltest du nicht. Dies wäre eher redundant ( Update ).

Sie sollten verwenden, checkNotNullum schnell zu versagen . Ohne sie können Sie ein Illegales nullan eine andere Methode übergeben, die es weiterleitet, und so weiter und so fort, wo es schließlich fehlschlägt. Dann können Sie etwas Glück brauchen, um herauszufinden, dass eigentlich die allererste Methode abgelehnt haben sollte null.


Die Antwort von yshavit erwähnt einen wichtigen Punkt: Es ist schlecht, einen illegalen Wert zu übergeben, aber es ist noch schlimmer, ihn zu speichern und später weiterzugeben.

Aktualisieren

Tatsächlich,

 checkNotNull(myObject).getAnything()

Dies ist auch sinnvoll, da Sie klar Ihre Absicht zum Ausdruck bringen, keine Nullen zu akzeptieren. Ohne sie könnte jemand denken, dass Sie den Scheck vergessen haben und ihn in so etwas umwandeln

 myObject != null ? myObject.getAnything() : somethingElse

OTOH, ich denke nicht, dass der Scheck die Ausführlichkeit wert ist. In einer besseren Sprache würde das Typsystem die Nullbarkeit berücksichtigen und uns etwas semantischen Zucker geben

 myObject!!.getAnything()                    // checkNotNull
 myObject?.getAnything()                     // safe call else null
 myObject?.getAnything() ?: somethingElse    // safe call else somethingElse

für nullable myObject, während die Standard-Punktsyntax nur zulässig ist, wenn myObjectbekannt ist, dass sie nicht null ist.

Maaartinus
quelle
Das ist also so, als würde man am Anfang einer Methode überprüfen, ob eine Voraussetzung für die Methode gültig ist?
Juru
Genau. Wenn Sie sofort nach illegalen Argumenten suchen, müssen Sie sie später nicht mehr suchen. Außerdem sieht jeder, der sich die Methode ansieht, was verboten ist (sicher, es sollte dokumentiert werden ...).
Maaartinus
@Juru: Ja, deshalb ist es in der PreconditionsKlasse!
ColinD
5

Ich habe diesen ganzen Thread vor ein paar Minuten gelesen. Trotzdem war ich verwirrt, warum wir verwenden sollten checkNotNull. Dann schauen Sie sich das Precondition Class Doc von Guava an und ich habe bekommen, was ich erwartet hatte. Übermäßiger Gebrauch checkNotNullbeeinträchtigt die Leistung definitiv.

Meiner Meinung nach ist eine checkNotNullMethode für die Datenvalidierung erforderlich, die vom Benutzer direkt oder von der End-API zur Benutzerinteraktion erfolgt. Es sollte nicht in allen Methoden der internen API verwendet werden, da Sie mit dieser Ausnahme keine Ausnahme stoppen können, sondern Ihre interne API korrigieren müssen, um Ausnahmen zu vermeiden.

Laut DOC: Link

Verwenden von checkNotNull:

public static double sqrt(double value) {
     Preconditions.checkArgument(value >= 0.0, "negative value: %s", value);
     // calculate the square root
}

Warnung vor Leistung

Das Ziel dieser Klasse ist es, die Lesbarkeit von Code zu verbessern. Unter bestimmten Umständen kann dies jedoch zu erheblichen Leistungskosten führen. Denken Sie daran, dass alle Parameterwerte für die Nachrichtenerstellung sorgfältig berechnet werden müssen und dass auch Autoboxing und Varargs-Array-Erstellung stattfinden können, selbst wenn die Vorbedingungsprüfung dann erfolgreich ist (wie dies in der Produktion fast immer der Fall sein sollte). Unter bestimmten Umständen können diese verschwendeten CPU-Zyklen und Zuordnungen zu einem echten Problem führen. Leistungsabhängige Vorbedingungsprüfungen können immer in das übliche Formular konvertiert werden:

if (value < 0.0) {
     throw new IllegalArgumentException("negative value: " + value);
}
iamcrypticcoder
quelle
1
Geben Sie -1 an, sorry: Ich empfehle nicht, dies für die Validierung von Benutzerdaten zu verwenden, da Sie dem Benutzer eine benutzerfreundlichere nicht-technische Fehlermeldung geben möchten. Auch Laufzeitausnahmen sind nicht vorgesehen, wenn die unregelmäßige Situation zu erwarten ist. Von einem Benutzer kann erwartet werden, dass er ab und zu einen Fehler macht. Ich würde vorschlagen, Methodenargumente in öffentlichen Methoden zu überprüfen, aber nicht in privaten Methoden. Das frühere Erkennen von Programmierfehlern ist mehr wert als eine vorzeitige Optimierung. Ersetzen Sie die Voraussetzungsprüfung nur, wenn Sie feststellen, dass es sich um ein tatsächliches Leistungsproblem handelt.
Henno Vermeulen
@HennoVermeulen Wie Sie sagten "Überprüfen von Methodenargumenten in öffentlichen Methoden", sagte ich auch "Es sollte nicht in jeder Methode der internen API verwendet werden". Vielen Dank
iamcrypticcoder
Upvoted für die Heads-up auf Leistung, aber wie andere gesagt haben, kein guter Ansatz zur Benutzervalidierung!
Tullochgorum