Null-Check-Kette vs. NullPointerException abfangen

118

Ein Webdienst gibt ein riesiges XML zurück und ich muss auf tief verschachtelte Felder davon zugreifen. Beispielsweise:

return wsObject.getFoo().getBar().getBaz().getInt()

Das Problem ist , dass getFoo(), getBar(), getBaz()können alle zurück null.

Wenn ich jedoch nullin allen Fällen nachschaue, wird der Code sehr ausführlich und schwer zu lesen. Außerdem kann es sein, dass ich die Schecks für einige Felder verpasse.

if (wsObject.getFoo() == null) return -1;
if (wsObject.getFoo().getBar() == null) return -1;
// maybe also do something with wsObject.getFoo().getBar()
if (wsObject.getFoo().getBar().getBaz() == null) return -1;
return wsObject.getFoo().getBar().getBaz().getInt();

Ist es akzeptabel zu schreiben?

try {
    return wsObject.getFoo().getBar().getBaz().getInt();
} catch (NullPointerException ignored) {
    return -1;
}

oder wäre das ein Antimuster?

David Frank
quelle
29
Die nullÜberprüfungen würden mir nicht so viel ausmachen, da wsObject.getFoo().getBar().getBaz().getInt()es bereits nach Code riecht. Lesen Sie, was das "Gesetz von Demeter" ist, und ziehen Sie es vor, Ihren Code entsprechend umzugestalten. Dann ist auch das Problem mit den nullPrüfungen verschwunden. Und denken Sie darüber nach, zu verwenden Optional.
Tom
9
Was ist mit XPath und überlässt es ihrer Bewertung?
Joop Eggen
15
Dieser Code wird wahrscheinlich von generiert wsdl2java, der das Gesetz von Demeter nicht respektiert.
Adrian Cox

Antworten:

143

Das Fangen NullPointerExceptionist sehr problematisch, da es fast überall passieren kann. Es ist sehr einfach, einen Fehler zu beheben, ihn versehentlich zu erkennen und so fortzufahren, als ob alles normal wäre, wodurch ein echtes Problem verborgen bleibt. Es ist so schwierig damit umzugehen, deshalb ist es am besten, es ganz zu vermeiden. (Denken Sie beispielsweise an das automatische Entpacken einer Null Integer.)

Ich schlage vor, dass Sie Optionalstattdessen die Klasse verwenden. Dies ist häufig der beste Ansatz, wenn Sie mit vorhandenen oder fehlenden Werten arbeiten möchten.

Damit könnten Sie Ihren Code folgendermaßen schreiben:

public Optional<Integer> m(Ws wsObject) {
    return Optional.ofNullable(wsObject.getFoo()) // Here you get Optional.empty() if the Foo is null
        .map(f -> f.getBar()) // Here you transform the optional or get empty if the Bar is null
        .map(b -> b.getBaz())
        .map(b -> b.getInt());
        // Add this if you want to return null instead of an empty optional if any is null
        // .orElse(null);
        // Or this if you want to throw an exception instead
        // .orElseThrow(SomeApplicationException::new);
}

Warum optional?

Die Verwendung von Optionals anstelle von nullWerten, die möglicherweise nicht vorhanden sind, macht diese Tatsache für die Leser sehr sichtbar und klar, und das Typsystem stellt sicher, dass Sie sie nicht versehentlich vergessen.

Sie erhalten auch Zugriff auf Methoden zum bequemeren Arbeiten mit solchen Werten, wie z map und orElse.


Ist Abwesenheit gültig oder Fehler?

Denken Sie aber auch darüber nach, ob es ein gültiges Ergebnis für die Zwischenmethoden ist, null zurückzugeben, oder ob dies ein Zeichen für einen Fehler ist. Wenn es sich immer um einen Fehler handelt, ist es wahrscheinlich besser, eine Ausnahme auszulösen, als einen speziellen Wert zurückzugeben, oder dass die Zwischenmethoden selbst eine Ausnahme auslösen.


Vielleicht mehr Optionen?

Wenn andererseits fehlende Werte aus den Zwischenmethoden gültig sind, können Sie möglicherweise zu wechseln Optional für sie s ?

Dann könnten Sie sie so verwenden:

public Optional<Integer> mo(Ws wsObject) {
    return wsObject.getFoo()
        .flatMap(f -> f.getBar())
        .flatMap(b -> b.getBaz())
        .flatMap(b -> b.getInt());        
}

Warum nicht optional?

Der einzige Grund, warum ich daran denken kann, nicht zu verwenden, Optionalist, wenn dies ein wirklich leistungskritischer Teil des Codes ist und sich herausstellt, dass der Aufwand für die Speicherbereinigung ein Problem darstellt. Dies liegt daran Optional, dass bei jeder Ausführung des Codes einige Objekte zugewiesen werden und die VM diese möglicherweise nicht optimieren kann. In diesem Fall könnten Ihre ursprünglichen Wenn-Tests besser sein.

Lii
quelle
Sehr gute Antwort. Es sollte erwähnt werden, dass Sie, wenn es andere mögliche Fehlerbedingungen gibt und Sie zwischen diesen unterscheiden müssen, Trystattdessen verwenden können Optional. Obwohl Trydie Java-API keine enthält , gibt es viele Bibliotheken , die eine bereitstellen, z. B. javaslang.io , github.com/bradleyscollins/try4j , functionjava.org oder github.com/jasongoodwin/better-java-monads
Landei,
8
FClass::getBaretc wäre kürzer.
Boris die Spinne
1
@ BoristheSpider: Vielleicht ein bisschen. Aber ich bevorzuge normalerweise Lambdas gegenüber Methodenreferenzen, da die Klassennamen oft viel länger sind und ich finde, dass Lambdas etwas leichter zu lesen sind.
Lii
6
@Lii fair genug, aber beachten Sie, dass eine Methodenreferenz möglicherweise ein kleines bisschen schneller ist, da ein Lambda möglicherweise komplexere Konstrukte zur Kompilierungszeit erfordert. Das Lambda erfordert die Generierung einer staticMethode, die eine sehr geringe Strafe nach sich zieht.
Boris die Spinne
1
@Lii Ich finde Methodenreferenzen tatsächlich sauberer und aussagekräftiger, selbst wenn sie etwas länger sind.
Shmosel
14

Ich schlage vor, darüber nachzudenken Objects.requireNonNull(T obj, String message). Sie können Ketten mit einer detaillierten Nachricht für jede Ausnahme erstellen, z

requireNonNull(requireNonNull(requireNonNull(
    wsObject, "wsObject is null")
        .getFoo(), "getFoo() is null")
            .getBar(), "getBar() is null");

Ich würde vorschlagen, dass Sie keine speziellen Rückgabewerte verwenden, wie z -1. Das ist kein Java-Stil. Java hat den Mechanismus der Ausnahmen entwickelt, um diesen altmodischen Weg zu vermeiden, der aus der C-Sprache stammt.

Werfen NullPointerExceptionist auch nicht die beste Option. Sie können Ihre eigene Ausnahme angeben (indem Sie sie aktivieren, um sicherzustellen, dass sie von einem Benutzer behandelt wird, oder deaktivieren , um sie auf einfachere Weise zu verarbeiten) oder eine bestimmte Ausnahme vom von Ihnen verwendeten XML-Parser verwenden.

Andrew Tobilko
quelle
1
Objects.requireNonNullwirft schließlich NullPointerException. Das macht die Situation also nicht anders alsreturn wsObject.getFoo().getBar().getBaz().getInt()
Arka Ghosh
1
@ArkaGhosh, auch es vermeidet eine Menge ifs, wie OP zeigte
Andrew Tobilko
4
Dies ist die einzig vernünftige Lösung. Alle anderen empfehlen, Ausnahmen für die Flusskontrolle zu verwenden, bei denen es sich um einen Codegeruch handelt. Nebenbei bemerkt: Ich betrachte die vom OP durchgeführte Methodenverkettung ebenfalls als Geruch. Wenn er mit drei lokalen Variablen und den entsprechenden Wenns arbeiten würde, wäre die Situation viel klarer. Ich denke auch, dass das Problem tiefer liegt als nur eine NPE zu umgehen: OP sollte sich fragen, warum die Getter null zurückgeben können. Was bedeutet null? Vielleicht wäre ein Nullobjekt besser? Oder mit einer sinnvollen Ausnahme in einem Getter abstürzen? Grundsätzlich ist alles besser als Ausnahmen für die Flusskontrolle.
Marius K.
1
Der bedingungslose Rat, Ausnahmen zu verwenden, um das Fehlen eines gültigen Rückgabewerts anzuzeigen, ist nicht sehr gut. Ausnahmen sind nützlich, wenn eine Methode auf eine Weise fehlschlägt, die für den Aufrufer schwer wiederherzustellen ist und die in einer try-catch-Anweisung in einem anderen Teil des Programms besser behandelt wird. Um einfach das Fehlen eines Rückgabewerts anzuzeigen, ist es besser, die OptionalKlasse zu verwenden oder einen Nullwert zurückzugebenInteger
Lii
6

Unter der Annahme, dass die Klassenstruktur tatsächlich außerhalb unserer Kontrolle liegt, wie es der Fall zu sein scheint, halte ich es für eine vernünftige Lösung, die in der Frage vorgeschlagene NPE zu fangen, es sei denn, die Leistung ist ein Hauptanliegen. Eine kleine Verbesserung könnte darin bestehen, die Wurf- / Fanglogik zu verpacken, um Unordnung zu vermeiden:

static <T> T get(Supplier<T> supplier, T defaultValue) {
    try {
        return supplier.get();
    } catch (NullPointerException e) {
        return defaultValue;
    }
}

Jetzt können Sie einfach tun:

return get(() -> wsObject.getFoo().getBar().getBaz().getInt(), -1);
shmosel
quelle
return get(() -> wsObject.getFoo().getBar().getBaz().getInt(), "");gibt keinen Fehler in der Kompilierungszeit, der problematisch sein könnte.
Philippe Gioseffi
5

Wie bereits von Tom im Kommentar erwähnt,

Die folgende Aussage verstößt gegen das Gesetz von Demeter .

wsObject.getFoo().getBar().getBaz().getInt()

Was Sie wollen, ist intund Sie können es bekommen Foo. Das Gesetz von Demeter sagt, rede niemals mit den Fremden . Für Ihren Fall können Sie die eigentliche Implementierung unter der Haube von Foound verstecken Bar.

Jetzt können Sie erstellen Methode Foozu holen intaus Baz. Letztendlich Foowird Barund in können Barwir zugreifen, Intohne Bazdirekt ausgesetzt zu sein Foo. Daher werden Nullprüfungen wahrscheinlich in verschiedene Klassen unterteilt, und nur die erforderlichen Attribute werden von den Klassen gemeinsam genutzt.

CoderCroc
quelle
4
Es ist fraglich, ob es gegen das Gesetz von Demeter verstößt, da das WsObject wahrscheinlich nur eine Datenstruktur ist. Siehe hier: stackoverflow.com/a/26021695/1528880
DerM
2
@DerM Ja, das ist möglich, aber da OP bereits etwas hat, das seine XML-Datei analysiert, kann er auch darüber nachdenken, geeignete Modellklassen für erforderliche Tags zu erstellen, damit die Analysebibliothek sie zuordnen kann. Diese Modellklassen enthalten dann die Logik für die nullÜberprüfung ihrer eigenen Sub-Tags.
Tom
4

Meine Antwort geht fast in die gleiche Zeile wie @janki, aber ich möchte das Code-Snippet wie folgt leicht ändern:

if (wsObject.getFoo() != null && wsObject.getFoo().getBar() != null && wsObject.getFoo().getBar().getBaz() != null) 
   return wsObject.getFoo().getBar().getBaz().getInt();
else
   return something or throw exception;

Sie können auch eine Nullprüfung hinzufügen wsObject, wenn die Wahrscheinlichkeit besteht, dass dieses Objekt null ist.

Arka Ghosh
quelle
4

Sie sagen, dass einige Methoden "möglicherweise zurückkehren null", sagen aber nicht, unter welchen Umständen sie zurückkehren null. Sie sagen, Sie fangen das, NullPointerExceptionaber Sie sagen nicht, warum Sie es fangen. Dieser Mangel an Informationen deutet darauf hin, dass Sie nicht genau wissen, wofür Ausnahmen gelten und warum sie der Alternative überlegen sind.

Stellen Sie sich eine Klassenmethode vor, die eine Aktion ausführen soll, die Methode kann jedoch aufgrund von Umständen, die außerhalb ihrer Kontrolle liegen (was in der Tat für alle Methoden in Java der Fall ist), nicht garantieren , dass sie die Aktion ausführt . Wir nennen diese Methode und sie kehrt zurück. Der Code, der diese Methode aufruft, muss wissen, ob sie erfolgreich war. Wie kann es wissen? Wie kann es strukturiert werden, um mit den beiden Möglichkeiten von Erfolg oder Misserfolg umzugehen?

Mit Ausnahmen können wir Methoden schreiben, die als Nachbedingung erfolgreich sind . Wenn die Methode zurückgegeben wird, war sie erfolgreich. Wenn es eine Ausnahme auslöst, ist es fehlgeschlagen. Dies ist ein großer Gewinn für die Klarheit. Wir können Code schreiben, der den normalen Erfolgsfall klar verarbeitet, und den gesamten Fehlerbehandlungscode in catchKlauseln verschieben. Es stellt sich häufig heraus, dass die Details darüber, wie oder warum eine Methode nicht erfolgreich war, für den Aufrufer nicht wichtig sind, sodass dieselbe catchKlausel für die Behandlung mehrerer Arten von Fehlern verwendet werden kann. Und es kommt oft vor, dass ein Verfahren nicht zu fangen Ausnahmen braucht sich überhaupt , kann aber nur erlauben sie zu propagieren seine Anrufer. Ausnahmen aufgrund von Programmfehlern sind in dieser letzteren Klasse; Nur wenige Methoden können angemessen reagieren, wenn ein Fehler vorliegt.

Also diese Methoden, die zurückkehren null.

  • Zeigt ein nullWert einen Fehler in Ihrem Code an? Wenn dies der Fall ist, sollten Sie die Ausnahme überhaupt nicht abfangen. Und Ihr Code sollte nicht versuchen, sich selbst zu erraten. Schreiben Sie einfach, was klar und prägnant ist, unter der Annahme, dass es funktionieren wird. Ist eine Kette von Methodenaufrufen klar und prägnant? Dann benutze sie einfach.
  • Zeigt ein nullWert eine ungültige Eingabe in Ihr Programm an? Wenn dies der Fall ist, NullPointerExceptionist a keine geeignete Ausnahme, da es herkömmlicherweise für die Anzeige von Fehlern reserviert ist. Sie möchten wahrscheinlich eine benutzerdefinierte Ausnahme auslösen, die von IllegalArgumentException(wenn Sie eine nicht aktivierte Ausnahme möchten ) oder IOException(wenn Sie eine aktivierte Ausnahme möchten) abgeleitet ist. Muss Ihr Programm detaillierte Syntaxfehlermeldungen bereitstellen, wenn eine ungültige Eingabe vorliegt? Wenn ja, überprüfen Sie jede Methode auf anull Rückgabewert dann eine entsprechende Diagnoseausnahme auslösen. Wenn Ihr Programm keine detaillierte Diagnose bereitstellen muss, NullPointerExceptionist es am klarsten und präzisesten, die Methodenaufrufe miteinander zu verketten, alle abzufangen und dann Ihre benutzerdefinierte Ausnahme auszulösen.

Eine der Antworten behauptet, dass die verketteten Methodenaufrufe die verletzen Gesetz von Demeterund daher schlecht sind. Diese Behauptung ist falsch.

  • Wenn es um Programmdesign geht, gibt es eigentlich keine absoluten Regeln darüber, was gut und was schlecht ist. Es gibt nur Heuristiken: Regeln, die die meiste Zeit (sogar fast immer) richtig sind. Ein Teil der Programmierkenntnisse besteht darin, zu wissen, wann es in Ordnung ist, solche Regeln zu brechen. Eine knappe Behauptung, dass "dies gegen Regel X ist ", ist also überhaupt keine Antwort. Ist dies eine der Situationen , in denen die Regel sollte gebrochen werden?
  • Das Gesetz von Demeter ist wirklich eine Regel über das Design von APIs oder Klassenschnittstellen. Beim Entwerfen von Klassen ist es nützlich, eine zu haben Hierarchie von Abstraktionen zu haben. Sie haben Klassen auf niedriger Ebene, die die Sprachprimitive verwenden, um Operationen direkt auszuführen und Objekte in einer Abstraktion darzustellen, die höher als die Sprachprimitive ist. Sie haben Klassen mittlerer Ebene, die an Klassen niedriger Ebene delegieren und Operationen und Darstellungen auf einer höheren Ebene als Klassen niedriger Ebene implementieren. Sie haben Klassen auf hoher Ebene, die an Klassen auf mittlerer Ebene delegieren und Operationen und Abstraktionen auf höherer Ebene implementieren. (Ich habe hier nur über drei Abstraktionsebenen gesprochen, aber mehr sind möglich). Auf diese Weise kann sich Ihr Code in geeigneten Abstraktionen auf jeder Ebene ausdrücken und so die Komplexität verbergen. Die Begründung für die Gesetz von DemeterWenn Sie eine Kette von Methodenaufrufen haben, bedeutet dies, dass Sie eine Klasse auf hoher Ebene haben, die über eine Klasse auf mittlerer Ebene direkt mit Details auf niedriger Ebene umgeht, und dass Ihre Klasse auf mittlerer Ebene daher keine abstrakte Operation auf mittlerer Ebene bereitgestellt hat dass die hohe Klasse braucht. Dies scheint jedoch nicht der Fall zu sein: Sie haben die Klassen in der Kette der Methodenaufrufe nicht entworfen, sie sind das Ergebnis eines automatisch generierten XML-Serialisierungscodes (richtig?), Und die Kette der Aufrufe ist nicht absteigend durch eine Abstraktionshierarchie, weil sich das deserialisierte XML alle auf derselben Ebene der Abstraktionshierarchie befindet (richtig?)?
Raedwald
quelle
3

Um die Lesbarkeit zu verbessern, möchten Sie möglicherweise mehrere Variablen verwenden, z

Foo theFoo;
Bar theBar;
Baz theBaz;

theFoo = wsObject.getFoo();

if ( theFoo == null ) {
  // Exit.
}

theBar = theFoo.getBar();

if ( theBar == null ) {
  // Exit.
}

theBaz = theBar.getBaz();

if ( theBaz == null ) {
  // Exit.
}

return theBaz.getInt();
JimmyB
quelle
Dies ist meiner Meinung nach weit weniger lesbar. Es übersät die Methode mit einer ganzen Reihe von Nullprüfungslogiken, die für die eigentliche Logik der Methode völlig irrelevant sind.
Developer102938
2

Nicht fangen NullPointerException. Sie wissen nicht, woher es kommt (ich weiß, dass es in Ihrem Fall nicht wahrscheinlich ist, aber vielleicht hat es etwas anderes geworfen) und es ist langsam. Sie möchten auf das angegebene Feld zugreifen und dafür muss jedes andere Feld nicht null sein. Dies ist ein perfekter Grund, jedes Feld zu überprüfen. Ich würde es wahrscheinlich in einem prüfen, wenn und dann eine Methode für die Lesbarkeit erstellen. Wie andere bereits betont haben, ist die Rückgabe von -1 sehr altmodisch, aber ich weiß nicht, ob Sie einen Grund dafür haben oder nicht (z. B. mit einem anderen System sprechen).

public int callService() {
    ...
    if(isValid(wsObject)){
        return wsObject.getFoo().getBar().getBaz().getInt();
    }
    return -1;
}


public boolean isValid(WsObject wsObject) {
    if(wsObject.getFoo() != null &&
        wsObject.getFoo().getBar() != null &&
        wsObject.getFoo().getBar().getBaz() != null) {
        return true;
    }
    return false;
}

Bearbeiten: Es ist fraglich, ob es gegen das Demeter-Gesetz verstößt, da das WsObject wahrscheinlich nur eine Datenstruktur ist (siehe https://stackoverflow.com/a/26021695/1528880 ).

DerM
quelle
2

Wenn Sie den Code nicht umgestalten möchten und Java 8 verwenden können, können Sie Methodenreferenzen verwenden.

Eine einfache Demo zuerst (entschuldigen Sie die statischen inneren Klassen)

public class JavaApplication14 
{
    static class Baz
    {
        private final int _int;
        public Baz(int value){ _int = value; }
        public int getInt(){ return _int; }
    }
    static class Bar
    {
        private final Baz _baz;
        public Bar(Baz baz){ _baz = baz; }
        public Baz getBar(){ return _baz; }   
    }
    static class Foo
    {
        private final Bar _bar;
        public Foo(Bar bar){ _bar = bar; }
        public Bar getBar(){ return _bar; }   
    }
    static class WSObject
    {
        private final Foo _foo;
        public WSObject(Foo foo){ _foo = foo; }
        public Foo getFoo(){ return _foo; }
    }
    interface Getter<T, R>
    {
        R get(T value);
    }

    static class GetterResult<R>
    {
        public R result;
        public int lastIndex;
    }

    /**
     * @param args the command line arguments
     */
    public static void main(String[] args) 
    {
        WSObject wsObject = new WSObject(new Foo(new Bar(new Baz(241))));
        WSObject wsObjectNull = new WSObject(new Foo(null));

        GetterResult<Integer> intResult
                = getterChain(wsObject, WSObject::getFoo, Foo::getBar, Bar::getBar, Baz::getInt);

        GetterResult<Integer> intResult2
                = getterChain(wsObjectNull, WSObject::getFoo, Foo::getBar, Bar::getBar, Baz::getInt);


        System.out.println(intResult.result);
        System.out.println(intResult.lastIndex);

        System.out.println();
        System.out.println(intResult2.result);
        System.out.println(intResult2.lastIndex);

        // TODO code application logic here
    }

    public static <R, V1, V2, V3, V4> GetterResult<R>
            getterChain(V1 value, Getter<V1, V2> g1, Getter<V2, V3> g2, Getter<V3, V4> g3, Getter<V4, R> g4)
            {
                GetterResult result = new GetterResult<>();

                Object tmp = value;


                if (tmp == null)
                    return result;
                tmp = g1.get((V1)tmp);
                result.lastIndex++;


                if (tmp == null)
                    return result;
                tmp = g2.get((V2)tmp);
                result.lastIndex++;

                if (tmp == null)
                    return result;
                tmp = g3.get((V3)tmp);
                result.lastIndex++;

                if (tmp == null)
                    return result;
                tmp = g4.get((V4)tmp);
                result.lastIndex++;


                result.result = (R)tmp;

                return result;
            }
}

Ausgabe

241
4

null
2

Die Schnittstelle Getterist nur eine funktionale Schnittstelle. Sie können eine beliebige Entsprechung verwenden.
GetterResultKlasse, Accessororen aus Gründen der Klarheit entfernt, halten Sie das Ergebnis der Getter-Kette, falls vorhanden, oder den Index des zuletzt aufgerufenen Getter.

Die Methode getterChainist ein einfacher Code, der automatisch (oder bei Bedarf manuell) generiert werden kann.
Ich habe den Code so strukturiert, dass der sich wiederholende Block offensichtlich ist.


Dies ist keine perfekte Lösung, da Sie immer noch eine Überladung getterChainpro Anzahl von Gettern definieren müssen.

Ich würde stattdessen den Code umgestalten, aber wenn dies nicht möglich ist und Sie häufig lange Getterketten verwenden, können Sie eine Klasse mit Überladungen erstellen, die zwischen 2 und beispielsweise 10 Getter liegen.

Margaret Bloom
quelle
2

Wie andere gesagt haben, ist die Einhaltung des Demeter-Gesetzes definitiv Teil der Lösung. Ein anderer Teil besteht, wo immer möglich, darin, diese verketteten Methoden so zu ändern, dass sie nicht zurückkehren können null. Sie können die Rückkehr vermeiden, nullindem Sie stattdessen ein leeres String, ein leeres Collectionoder ein anderes Dummy-Objekt zurückgeben, das bedeutet oder tut, was auch immer der Aufrufer tun würde null.

Kevin Krumwiede
quelle
2

Ich möchte eine Antwort hinzufügen, die sich auf die Bedeutung des Fehlers konzentriert . Eine Null-Ausnahme an sich liefert keinen vollständigen Fehler. Ich würde daher raten, nicht direkt mit ihnen umzugehen.

Es gibt Tausende von Fällen, in denen Ihr Code schief gehen kann: Keine Verbindung zur Datenbank, E / A-Ausnahme, Netzwerkfehler ... Wenn Sie sie einzeln behandeln (wie hier die Nullprüfung), wäre dies zu mühsam.

Im Code:

wsObject.getFoo().getBar().getBaz().getInt();

Selbst wenn Sie wissen, welches Feld null ist, haben Sie keine Ahnung, was schief geht. Vielleicht ist Bar null, aber wird es erwartet? Oder ist es ein Datenfehler? Denken Sie an Leute, die Ihren Code lesen

Wie in der Antwort von xenteros würde ich vorschlagen, eine benutzerdefinierte, nicht aktivierte Ausnahme zu verwenden . Zum Beispiel in dieser Situation: Foo kann null sein (gültige Daten), aber Bar und Baz sollten niemals null sein (ungültige Daten)

Der Code kann neu geschrieben werden:

void myFunction()
{
    try 
    {
        if (wsObject.getFoo() == null)
        {
          throw new FooNotExistException();
        }

        return wsObject.getFoo().getBar().getBaz().getInt();
    }
    catch (Exception ex)
    {
        log.error(ex.Message, ex); // Write log to track whatever exception happening
        throw new OperationFailedException("The requested operation failed")
    }
}


void Main()
{
    try
    {
        myFunction();
    }
    catch(FooNotExistException)
    {
        // Show error: "Your foo does not exist, please check"
    }
    catch(OperationFailedException)
    {
        // Show error: "Operation failed, please contact our support"
    }
}
Hoàng Long
quelle
Nicht aktivierte Ausnahmen weisen darauf hin, dass der Programmierer die API missbraucht. Externe Probleme wie "Verbindung zur Datenbank nicht möglich, E / A-Ausnahme, Netzwerkfehler" sollten durch aktivierte Ausnahmen angezeigt werden.
Kevin Krumwiede
Es kommt wirklich auf die Bedürfnisse des Anrufers an. Überprüfte Ausnahmehilfe, da Sie gezwungen sind, den Fehler zu verarbeiten. In anderen Fällen ist dies jedoch nicht erforderlich und kann den Code verschmutzen. Sie haben beispielsweise eine IOException in Ihrer Datenebene. Werden Sie diese in die Präsentationsebene werfen? Das würde bedeuten, dass Sie die Ausnahme abfangen und jeden Anrufer erneut angreifen müssen. Ich würde es vorziehen, die IOException mit einer benutzerdefinierten BusinessException mit einer relevanten Nachricht zu versehen und sie durch den Stacktrace laufen zu lassen, bis ein globaler Filter sie abfängt und dem Benutzer die Nachricht anzeigt.
Hoàng Long
Anrufer müssen geprüfte Ausnahmen nicht abfangen und erneut auslösen, sondern nur deklarieren, dass sie ausgelöst werden.
Kevin Krumwiede
@ KevinKrumwiede: Sie haben Recht, wir müssen nur die Ausnahme deklarieren, die ausgelöst werden soll. Wir müssen jedoch noch erklären. Bearbeiten: Bei einem zweiten Blick gibt es eine ganze Reihe von Debatten über die Verwendung von geprüften und nicht geprüften Ausnahmen (z . B. programmers.stackexchange.com/questions/121328/… ).
Hoàng Long
2

NullPointerException ist eine Laufzeitausnahme, daher wird im Allgemeinen nicht empfohlen, sie abzufangen, sondern zu vermeiden.

Sie müssen die Ausnahme abfangen, wo immer Sie die Methode aufrufen möchten (oder sie wird den Stapel weitergeben). Wenn Sie in Ihrem Fall mit diesem Ergebnis mit dem Wert -1 weiterarbeiten können und sicher sind, dass es sich nicht ausbreitet, weil Sie keine der "Teile" verwenden, die möglicherweise null sind, dann scheint es mir richtig zu sein fang es

Bearbeiten:

Ich stimme der späteren Antwort von @xenteros zu. Es ist besser, eine eigene Ausnahme zu starten, anstatt -1 zurückzugeben, die Sie beispielsweise aufrufen können InvalidXMLException.

SCouto
quelle
3
Was meinst du mit "egal ob du es fängst, es kann sich auf andere Teile des Codes ausbreiten"?
Hulk
Wenn die Null in diesem Satz steht, wsObject.getFoo (). In späteren Teilen des Codes führen Sie diese Abfrage erneut aus oder verwenden wsObject.getFoo (). GetBar () (zum Beispiel), wodurch erneut eine NullPointerException ausgelöst wird.
SCouto
Dies ist eine ungewöhnliche Formulierung für "Sie müssen die Ausnahme abfangen, wo immer Sie die Methode aufrufen möchten (oder sie wird den Stapel weitergeben)." wenn ich richtig verstehe. Ich stimme dem zu (und das kann ein Problem sein), ich finde den Wortlaut nur verwirrend.
Hulk
Ich werde es beheben, sorry, Englisch ist nicht meine Muttersprache, so dass dies manchmal passieren kann :) Danke
SCouto
2

Verfolge diesen Beitrag seit gestern.

Ich habe die Kommentare kommentiert / abgestimmt, die besagen, dass es schlecht ist, NPE zu fangen. Hier ist, warum ich das getan habe.

package com.todelete;

public class Test {
    public static void main(String[] args) {
        Address address = new Address();
        address.setSomeCrap(null);
        Person person = new Person();
        person.setAddress(address);
        long startTime = System.currentTimeMillis();
        for (int i = 0; i < 1000000; i++) {
            try {
                System.out.println(person.getAddress().getSomeCrap().getCrap());
            } catch (NullPointerException npe) {

            }
        }
        long endTime = System.currentTimeMillis();
        System.out.println((endTime - startTime) / 1000F);
        long startTime1 = System.currentTimeMillis();
        for (int i = 0; i < 1000000; i++) {
            if (person != null) {
                Address address1 = person.getAddress();
                if (address1 != null) {
                    SomeCrap someCrap2 = address1.getSomeCrap();
                    if (someCrap2 != null) {
                        System.out.println(someCrap2.getCrap());
                    }
                }
            }
        }
        long endTime1 = System.currentTimeMillis();
        System.out.println((endTime1 - startTime1) / 1000F);
    }
}

  public class Person {
    private Address address;

    public Address getAddress() {
        return address;
    }

    public void setAddress(Address address) {
        this.address = address;
    }
}

package com.todelete;

public class Address {
    private SomeCrap someCrap;

    public SomeCrap getSomeCrap() {
        return someCrap;
    }

    public void setSomeCrap(SomeCrap someCrap) {
        this.someCrap = someCrap;
    }
}

package com.todelete;

public class SomeCrap {
    private String crap;

    public String getCrap() {
        return crap;
    }

    public void setCrap(String crap) {
        this.crap = crap;
    }
}

Ausgabe

3.216

0,002

Ich sehe hier einen klaren Gewinner. Wenn Schecks viel zu teuer sind, als eine Ausnahme zu erwischen. Ich habe diese Java-8-Methode gesehen. In Anbetracht der Tatsache, dass 70% der aktuellen Anwendungen immer noch auf Java-7 ausgeführt werden, füge ich diese Antwort hinzu.

Endeffekt Für alle geschäftskritischen Anwendungen ist die Handhabung von NPE kostspielig.

Neuer Benutzer
quelle
Drei zusätzliche Sekunden für eine Million Anfragen sind im schlimmsten Fall messbar, aber selbst in "geschäftskritischen Anwendungen" wäre dies selten ein Deal Breaker. Es gibt Systeme, bei denen das Hinzufügen von 3,2 Mikrosekunden zu einer Anforderung eine große Sache ist, und wenn Sie über ein solches System verfügen, sollten Sie auf jeden Fall sorgfältig über Ausnahmen nachdenken. Das Aufrufen eines Webdienstes und das Deserialisieren seiner Ausgabe gemäß der ursprünglichen Frage dauert jedoch wahrscheinlich viel länger, und die Sorge um die Leistung der Ausnahmebehandlung ist nebensächlich.
Jeroen Mostert
@JeroenMostert: 3 Sekunden pro Scheck / Million. Die Anzahl der Schecks erhöht also die Kosten
NewUser
Wahr. Trotzdem würde ich es immer noch als "Profil zuerst" betrachten. Sie benötigen mehr als 300 Schecks in einer Anfrage, bevor die Anfrage eine volle Millisekunde mehr dauert. Designüberlegungen würden meine Seele viel früher belasten.
Jeroen Mostert
@JeroenMostert: :) Einverstanden! Ich möchte es dem Programmierer mit dem Ergebnis überlassen und sie einen Anruf entgegennehmen lassen!
NewUser
1

Wenn Effizienz ein Problem ist, sollte die Option „Fang“ in Betracht gezogen werden. Wenn 'catch' nicht verwendet werden kann, weil es sich ausbreiten würde (wie von 'SCouto' erwähnt), verwenden Sie lokale Variablen, um mehrere Aufrufe von Methoden zu vermeiden getFoo(), getBar()und getBaz().

JEP
quelle
1

Es lohnt sich, eine eigene Ausnahme zu erstellen. Nennen wir es MyOperationFailedException. Sie können es werfen, anstatt einen Wert zurückzugeben. Das Ergebnis ist das gleiche - Sie beenden die Funktion, geben jedoch keinen fest codierten Wert -1 zurück, bei dem es sich um ein Java-Anti-Pattern handelt. In Java verwenden wir Ausnahmen.

try {
    return wsObject.getFoo().getBar().getBaz().getInt();
} catch (NullPointerException ignored) {
    throw new MyOperationFailedException();
}

BEARBEITEN:

Lassen Sie mich gemäß der Diskussion in den Kommentaren etwas zu meinen vorherigen Gedanken hinzufügen. In diesem Code gibt es zwei Möglichkeiten. Zum einen akzeptieren Sie null und zum anderen ist es ein Fehler.

Wenn es sich um einen Fehler handelt und dieser auftritt, können Sie Ihren Code mithilfe anderer Strukturen für Debugging-Zwecke debuggen, wenn Haltepunkte nicht ausreichen.

Wenn es akzeptabel ist, ist es Ihnen egal, wo diese Null angezeigt wurde. Wenn Sie dies tun, sollten Sie diese Anfragen definitiv nicht verketten.

Xenteros
quelle
2
Halten Sie es nicht für eine schlechte Idee, die Ausnahme zu unterdrücken? Wenn wir in Echtzeit die Spur einer Ausnahme verlieren, ist es wirklich schmerzhaft, herauszufinden, was zum Teufel los ist! Ich würde immer empfehlen, keine Verkettung zu verwenden. Das zweite Problem, das ich sehe, ist: Dieser Code kann zu einem bestimmten Zeitpunkt nicht gewährt werden. Welches der Ergebnisse war null?
NewUser
Nein, Ihre Ausnahme kann eine Nachricht enthalten, die sicherlich auf den Ort verweist, an den sie geworfen wurde. Ich bin damit einverstanden, dass Verkettung nicht die beste Lösung ist :)
Xenteros
3
Nein, es würde nur über die Zeilennummer sagen. Daher kann jeder Aufruf in der Kette zu einer Ausnahme führen.
NewUser
"Wenn es sich um einen Fehler handelt und dieser auftritt, können Sie Ihren Code debuggen" - nicht in der Produktion. Ich würde viel lieber wissen, WAS fehlgeschlagen ist, wenn ich nur ein Protokoll habe, als zu erraten, was passiert ist, um es zum Scheitern zu bringen. Mit diesem Rat (und diesem Code) wissen Sie wirklich nur, dass eines von vier Dingen null war, aber nicht welches oder warum.
VLAZ
1

Die Methode, die Sie haben, ist langwierig, aber sehr gut lesbar. Wenn ich ein neuer Entwickler wäre, der zu Ihrer Codebasis kommt, könnte ich ziemlich schnell sehen, was Sie tun. Die meisten anderen Antworten (einschließlich des Abfangens der Ausnahme) scheinen die Dinge nicht lesbarer zu machen, und einige machen sie meiner Meinung nach weniger lesbar.

Da Sie wahrscheinlich keine Kontrolle über die generierte Quelle haben und davon ausgehen, dass Sie hier und da wirklich nur auf einige tief verschachtelte Felder zugreifen müssen, würde ich empfehlen, jeden tief verschachtelten Zugriff mit einer Methode zu versehen.

private int getFooBarBazInt() {
    if (wsObject.getFoo() == null) return -1;
    if (wsObject.getFoo().getBar() == null) return -1;
    if (wsObject.getFoo().getBar().getBaz() == null) return -1;
    return wsObject.getFoo().getBar().getBaz().getInt();
}

Wenn Sie feststellen, dass Sie viele dieser Methoden schreiben oder versucht sind, diese öffentlichen statischen Methoden zu erstellen, würde ich ein separates Objektmodell erstellen, das nach Ihren Wünschen verschachtelt ist und nur die Felder enthält, die Sie interessieren, und aus dem Web konvertieren Services-Objektmodell für Ihr Objektmodell.

Wenn Sie mit einem Remote-Webdienst kommunizieren, ist es sehr typisch, eine "Remote-Domäne" und eine "Anwendungsdomäne" zu haben und zwischen beiden zu wechseln. Die Remotedomäne wird häufig durch das Webprotokoll eingeschränkt (Sie können beispielsweise keine Hilfsmethoden in einem reinen RESTful-Dienst hin und her senden, und tief verschachtelte Objektmodelle sind üblich, um mehrere API-Aufrufe zu vermeiden) und daher nicht ideal für die direkte Verwendung in dein Klient.

Beispielsweise:

public static class MyFoo {

    private int barBazInt;

    public MyFoo(Foo foo) {
        this.barBazInt = parseBarBazInt();
    }

    public int getBarBazInt() {
        return barBazInt;
    }

    private int parseFooBarBazInt(Foo foo) {
        if (foo() == null) return -1;
        if (foo().getBar() == null) return -1;
        if (foo().getBar().getBaz() == null) return -1;
        return foo().getBar().getBaz().getInt();
    }

}
Tempo
quelle
1
return wsObject.getFooBarBazInt();

durch Anwendung des Gesetzes von Demeter,

class WsObject
{
    FooObject foo;
    ..
    Integer getFooBarBazInt()
    {
        if(foo != null) return foo.getBarBazInt();
        else return null;
    }
}

class FooObject
{
    BarObject bar;
    ..
    Integer getBarBazInt()
    {
        if(bar != null) return bar.getBazInt();
        else return null;
    }
}

class BarObject
{
    BazObject baz;
    ..
    Integer getBazInt()
    {
        if(baz != null) return baz.getInt();
        else return null;
    }
}

class BazObject
{
    Integer myInt;
    ..
    Integer getInt()
    {
        return myInt;
    }
}
Khaled.K
quelle
0

Antwort geben, die sich von allen anderen zu unterscheiden scheint.

Ich empfehle Ihnen , zu überprüfen , für NULLin ifs.

Grund :

Wir sollten keine einzige Chance lassen, dass unser Programm abstürzt. NullPointer wird vom System generiert. Das Verhalten von vom System generierten Ausnahmen kann nicht vorhergesagt werden . Sie sollten Ihr Programm nicht in den Händen von System lassen, wenn Sie bereits die Möglichkeit haben, es selbst zu handhaben. Und setzen Sie den Ausnahmebehandlungsmechanismus für zusätzliche Sicherheit. !!

Versuchen Sie dies, um die Bedingungen zu überprüfen, damit Ihr Code leicht lesbar ist:

if (wsObject.getFoo() == null || wsObject.getFoo().getBar() == null || wsObject.getFoo().getBar().getBaz() == null) 
   return -1;
else 
   return wsObject.getFoo().getBar().getBaz().getInt();

EDIT:

Hier müssen Sie diese Werte speichern wsObject.getFoo(), wsObject.getFoo().getBar(), wsObject.getFoo().getBar().getBaz()in einigen Variablen. Ich mache es nicht, weil ich die Rückgabetypen dieser Funktionen nicht kenne.

Anregungen werden geschätzt .. !!

Janki Gadhiya
quelle
Haben Sie getFoo () für eine sehr zeitaufwändige Operation gehalten? Sie sollten zurückgegebene Werte in Variablen speichern, dies ist jedoch eine Verschwendung von Speicher. Ihre Methode ist perfekt für die C-Programmierung.
Xenteros
aber manchmal ist es besser, 1 Millisekunde zu spät zu sein, als dass das Programm bei xenteros abstürzt .. !!
Janki Gadhiya
getFoo () kann einen Wert von einem Server auf einem anderen Kontinent erhalten. Es kann jederzeit dauern: Minuten / Stunden ...
Xenteros
wsObjectenthält den vom Webservice zurückgegebenen Wert .. !! Der Service wird bereits aufgerufen und wsObjecterhält lange XMLDaten als Webservice-Antwort .. !! Es gibt also nichts Besseres als einen Server auf einem anderen Kontinent, denn es getFoo()handelt sich nur um ein Element, das die Getter-Methode erhält , nicht um einen Webservice-Aufruf. !! @xenteros
Janki Gadhiya
1
Nun, von den Namen der Getter würde ich annehmen, dass sie Foo-, Bar- und Baz-Objekte zurückgeben: P Ziehen Sie auch in Betracht, die erwähnte doppelte Sicherheit aus Ihrer Antwort zu entfernen. Ich denke nicht, dass es einen wirklichen Wert abgesehen von der Verschmutzung des Codes bietet. Mit vernünftigen lokalen Variablen und Nullprüfung haben wir mehr als genug getan, um die Richtigkeit des Codes sicherzustellen. Wenn eine Ausnahme auftreten kann, sollte sie als eine behandelt werden.
Marius K.
0

Ich habe eine Klasse namens geschrieben, mit Snagder Sie einen Pfad definieren können, um durch einen Baum von Objekten zu navigieren. Hier ist ein Beispiel für seine Verwendung:

Snag<Car, String> ENGINE_NAME = Snag.createForAndReturn(Car.class, String.class).toGet("engine.name").andReturnNullIfMissing();

Dies bedeutet, dass die Instanz die an sie übergebene Instanz ENGINE_NAMEeffektiv aufruft Car?.getEngine()?.getName()und zurückgibt, nullwenn eine Referenz zurückgegeben wird null:

final String name =  ENGINE_NAME.get(firstCar);

Es ist nicht auf Maven veröffentlicht, aber wenn jemand dies nützlich findet, ist es hier (natürlich ohne Garantie!)

Es ist ein bisschen einfach, aber es scheint den Job zu machen. Offensichtlich ist es mit neueren Versionen von Java und anderen JVM-Sprachen, die eine sichere Navigation unterstützen, veraltet oder Optional.

Reich
quelle