Chaîne de vérification des nuls et capture des NullPointerException

Question

Un service web renvoie un énorme XML et je dois accéder à des champs profondément imbriqués dans celui-ci. Par exemple :

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

Le problème est que getFoo() , getBar() , getBaz() que tous reviennent null .

Cependant, si je vérifie null dans tous les cas, le code devient très verbeux et difficile à lire. De plus, je peux manquer les vérifications de certains champs.

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();

Est-il acceptable d'écrire

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

ou cela serait-il considéré comme un anti-modèle ?

Answer 1

Attraper NullPointerException es un une chose vraiment problématique à faire puisqu'ils peuvent se produire presque partout. Il est très facile d'en attraper un, de l'attraper par accident et de continuer comme si tout était normal, cachant ainsi un réel problème. C'est tellement délicat à gérer qu'il vaut mieux l'éviter complètement. (Par exemple, pensez à l'auto-déballage d'un fichier null Integer .)

Je vous suggère d'utiliser le Optional à la place. C'est souvent la meilleure approche lorsque vous voulez travailler avec des valeurs qui sont soit présentes, soit absentes.

En utilisant cela, vous pourriez écrire votre code comme ceci :

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);
}

---

Pourquoi facultatif ?

Utilisation de Optional au lieu de null pour les valeurs qui pourraient être absentes rend ce fait très visible et clair pour les lecteurs, et le système de types s'assurera que vous ne l'oubliez pas accidentellement.

Vous avez également accès à des méthodes permettant de travailler avec ces valeurs de manière plus pratique, telles que map y orElse .

---

L'absence est-elle valable ou une erreur ?

Mais réfléchissez également à la question de savoir si le fait que les méthodes intermédiaires renvoient un résultat nul est un résultat valide ou si c'est le signe d'une erreur. Si c'est toujours une erreur, il est probablement préférable de lever une exception plutôt que de renvoyer une valeur spéciale ou que les méthodes intermédiaires elles-mêmes lèvent une exception.

---

Peut-être plus d'options ?

Si, par contre, les valeurs absentes des méthodes intermédiaires sont valides, vous pouvez peut-être passer à la méthode suivante Optional s pour eux aussi ?

Alors vous pourriez les utiliser comme ça :

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

---

Pourquoi pas en option ?

La seule raison à laquelle je pense pour ne pas utiliser Optional Le problème le plus important est qu'il s'agit d'une partie du code dont les performances sont vraiment critiques, et que la surcharge de la collecte des déchets s'avère être un problème. C'est parce que quelques Optional sont alloués à chaque fois que le code est exécuté, et la VM pourrait ne sera pas en mesure de les optimiser. Dans ce cas, vos tests if originaux sont peut-être plus appropriés.

Answer 2

Je suggère de considérer Objects.requireNonNull(T obj, String message) . Vous pouvez construire des chaînes avec un message détaillé pour chaque exception, par exemple

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

Je vous conseille de ne pas utiliser de valeurs de rendement spéciales, comme par exemple -1 . Ce n'est pas un style Java. Java a conçu le mécanisme des exceptions afin d'éviter cette manière démodée qui vient du langage C.

Lancer NullPointerException n'est pas non plus la meilleure option. Vous pourriez fournir votre propre exception (en la rendant vérifié pour garantir qu'il sera manipulé par un utilisateur ou non vérifié pour le traiter plus facilement) ou utiliser une exception spécifique de l'analyseur XML que vous utilisez.

Answer 3

En supposant que la structure de la classe est effectivement hors de notre contrôle, comme cela semble être le cas, je pense que la capture du NPE comme suggéré dans la question est en effet une solution raisonnable, à moins que la performance soit une préoccupation majeure. Une petite amélioration pourrait être d'envelopper la logique throw/catch pour éviter le désordre :

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

Maintenant, vous pouvez simplement faire :

return get(() -> wsObject.getFoo().getBar().getBaz().getInt(), -1);

Answer 4

Comme l'a déjà souligné Tom dans le commentaire,

La déclaration suivante désobéit à la Loi de Déméter ,

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

Ce que vous voulez, c'est int et vous pouvez l'obtenir à partir de Foo . Loi de Déméter dit, ne parlez jamais aux étrangers . Dans votre cas, vous pouvez cacher l'implémentation réelle sous le capot de l'application Foo y Bar .

Maintenant, vous pouvez créer une méthode dans Foo pour aller chercher int de Baz . En fin de compte, Foo aura Bar et en Bar nous pouvons accéder Int sans exposer Baz directement à Foo . Ainsi, les contrôles nuls sont probablement divisés en différentes classes et seuls les attributs requis seront partagés entre les classes.

Answer 5

Ma réponse va presque dans le même sens que celle de @janki, mais je voudrais modifier légèrement l'extrait de code comme ci-dessous :

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;

Vous pouvez ajouter un contrôle de nullité pour wsObject également, s'il y a une chance que cet objet soit nul.