Chaîne de vérification nulle vs capture NullPointerException

118

Un service Web renvoie un énorme XML et j'ai besoin d'accéder à ses champs profondément imbriqués. Par exemple:

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

Le problème est que getFoo(), getBar(), getBaz()peut tout retour null.

Cependant, si je vérifie nulldans tous les cas, le code devient très détaillé et difficile à lire. De plus, je peux manquer les contrôles pour 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?

David Frank
la source
29
Les nullvérifications ne me dérangeraient pas autant, car wsObject.getFoo().getBar().getBaz().getInt()c'est déjà une odeur de code. Lisez ce qu'est la «loi de Demeter» et préférez refactoriser votre code en conséquence. Ensuite, le problème avec les nullchèques disparaîtra également. Et pensez à utiliser Optional.
Tom
9
Qu'en est-il d'utiliser XPath et de le laisser à leur évaluation?
Joop Eggen
15
Ce code est probablement généré par wsdl2java, qui n'a aucun respect pour la loi de Déméter.
Adrian Cox

Réponses:

143

La capture NullPointerExceptionest une chose vraiment problématique à faire car elle peut se produire presque n'importe où. Il est très facile d'en obtenir un à partir d'un bug, de l'attraper par accident et de continuer comme si tout était normal, cachant ainsi un vrai problème. C'est tellement difficile à gérer, il est donc préférable d'éviter complètement. (Par exemple, pensez au déballage automatique d'un null Integer.)

Je vous suggère d'utiliser la Optionalclasse à la place. C'est souvent la meilleure approche lorsque vous souhaitez travailler avec des valeurs présentes ou absentes.

En utilisant cela, vous pouvez é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?

L'utilisation de Optionals au lieu de nullpour des valeurs qui pourraient être absentes rend ce fait très visible et clair pour les lecteurs, et le système de type s'assurera que vous ne l'oublierez pas accidentellement.

Vous avez également accès à des méthodes pour travailler plus facilement avec de telles valeurs, comme mapet orElse.


L'absence est-elle valide ou une erreur?

Mais pensez également à savoir s'il s'agit d'un résultat valide pour que les méthodes intermédiaires retournent null ou si cela est le signe d'une erreur. S'il s'agit toujours d'une erreur, il est probablement préférable de lancer une exception que de renvoyer une valeur spéciale, ou que les méthodes intermédiaires elles-mêmes lancent 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 à Optionals pour elles également?

Ensuite, vous pouvez les utiliser comme ceci:

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

Pourquoi pas facultatif?

La seule raison à laquelle je peux penser pour ne pas utiliser Optionalest si cela se trouve dans une partie vraiment critique des performances du code, et si la surcharge du garbage collection s'avère être un problème. En effet, quelques Optionalobjets sont alloués à chaque fois que le code est exécuté et la machine virtuelle peut ne pas être en mesure de les optimiser. Dans ce cas, vos tests if d'origine pourraient être meilleurs.

Lii
la source
Très bonne réponse. Il convient de mentionner que s'il existe d'autres conditions d'échec possibles et que vous devez les distinguer, vous pouvez utiliser à la Tryplace de Optional. Bien qu'il n'y Tryen ait pas dans l'API Java, de nombreuses bibliothèques en fournissent une, par exemple javaslang.io , github.com/bradleyscollins/try4j , functionaljava.org ou github.com/jasongoodwin/better-java-monads
Landei
8
FClass::getBaretc. serait plus court.
Boris the Spider
1
@BoristheSpider: Peut-être un peu. Mais je préfère généralement les lambdas aux méthodes refs car souvent les noms de classe sont beaucoup plus longs et je trouve les lambdas un peu plus faciles à lire.
Lii
6
@Lii assez juste, mais notez qu'une référence de méthode peut être un peu plus rapide, car un lambda peut nécessiter des constructions de temps de compilation plus complexes. Le lambda nécessitera la génération d'une staticméthode, ce qui entraînera une pénalité très mineure.
Boris the Spider
1
@Lii Je trouve en fait que les références de méthode sont plus propres et plus descriptives, même lorsqu'elles sont légèrement plus longues.
shmosel
14

Je suggère d'envisager Objects.requireNonNull(T obj, String message). Vous pouvez créer des chaînes avec un message détaillé pour chaque exception, comme

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

Je vous suggère de ne pas utiliser de valeurs de retour spéciales, comme -1. Ce n'est pas un style Java. Java a conçu le mécanisme des exceptions pour éviter cette méthode démodée issue du langage C.

Le lancer NullPointerExceptionn'est pas non plus la meilleure option. Vous pouvez fournir votre propre exception (en la vérifiant pour garantir qu'elle sera traitée par un utilisateur ou décochée pour la traiter plus facilement) ou utiliser une exception spécifique de l'analyseur XML que vous utilisez.

Andrew Tobilko
la source
1
Objects.requireNonNulljette finalement NullPointerException. Donc, cela ne rend pas la situation différente de cellereturn wsObject.getFoo().getBar().getBaz().getInt()
Arka Ghosh
1
@ArkaGhosh, cela évite également beaucoup de ifs comme l'a montré OP
Andrew Tobilko
4
C'est la seule solution sensée. Tous les autres conseillent d'utiliser des exceptions pour le contrôle de flux qui est une odeur de code. Par ailleurs, je considère que le chaînage de méthodes effectué par l'OP est également une odeur. S'il travaillait avec trois variables locales et les si correspondants, la situation serait beaucoup plus claire. Je pense également que le problème est plus profond que de simplement travailler autour d'un NPE: OP devrait se demander pourquoi les getters peuvent renvoyer null. Que signifie nul? Peut-être qu'un objet nul serait mieux? Ou planter dans un getter avec une exception significative? En gros, tout vaut mieux que les exceptions pour le contrôle de flux.
Marius K.
1
Le conseil inconditionnel d'utiliser des exceptions pour signaler l'absence d'une valeur de retour valide n'est pas très bon. Les exceptions sont utiles lorsqu'une méthode échoue d'une manière qui est difficile pour l'appelant à récupérer et qui est mieux gérée dans une instruction try-catch dans une autre partie du programme. Pour simplement signaler l'absence de valeur de retour, il est préférable d'utiliser la Optionalclasse, ou peut-être de renvoyer un Integer
nullable
6

En supposant que la structure de classe est en effet 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 ne soit une préoccupation majeure. Une petite amélioration pourrait être d'encapsuler la logique lancer / attraper pour éviter l'encombrement:

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);
shmosel
la source
return get(() -> wsObject.getFoo().getBar().getBaz().getInt(), "");ne donne pas d'erreur de compilation qui pourrait être problématique.
Philippe Gioseffi
5

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 intet vous pouvez l'obtenir Foo. Law of Demeter dit, ne parlez jamais aux étrangers . Pour votre cas, vous pouvez cacher la mise en œuvre réelle sous le capot de Fooet Bar.

Maintenant, vous pouvez créer méthode Foopour aller chercher à intpartir Baz. En fin de compte, Fooaura Baret dans Barnous pouvons accéder Intsans exposer Bazdirectement à Foo. Ainsi, les vérifications nulles sont probablement divisées en différentes classes et seuls les attributs requis seront partagés entre les classes.

CoderCroc
la source
4
Il est discutable s'il désobéit à la loi de Demeter puisque le WsObject n'est probablement qu'une structure de données. Voir ici: stackoverflow.com/a/26021695/1528880
DerM
2
@DerM Oui, c'est possible, mais comme OP a déjà quelque chose qui analyse son fichier XML, il peut également penser à créer des classes de modèle appropriées pour les balises requises, afin que la bibliothèque d'analyse puisse les mapper. Ensuite, ces classes de modèle contiennent la logique d'une nullvérification de ses propres sous-balises.
Tom
4

Ma réponse va presque dans la même ligne que @janki, mais j'aimerais 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 également ajouter une vérification de null wsObject, s'il y a une chance que cet objet soit nul.

Arka Ghosh
la source
4

Vous dites que certaines méthodes «peuvent revenir null» mais ne dites pas dans quelles circonstances elles reviennent null. Vous dites que vous attrapez le NullPointerExceptionmais vous ne dites pas pourquoi vous l'attrapez. Ce manque d'informations suggère que vous ne comprenez pas clairement à quoi servent les exceptions et pourquoi elles sont supérieures à l'alternative.

Considérez une méthode de classe qui est destinée à effectuer une action, mais la méthode ne peut pas garantir qu'elle exécutera l'action, en raison de circonstances indépendantes de sa volonté (ce qui est en fait le cas pour toutes les méthodes en Java ). Nous appelons cette méthode et elle revient. Le code qui appelle cette méthode doit savoir si elle a réussi. Comment peut-il savoir? Comment peut-il être structuré pour faire face aux deux possibilités, de succès ou d'échec?

En utilisant des exceptions, nous pouvons écrire des méthodes qui ont du succès en tant que post-condition . Si la méthode retourne, elle a réussi. S'il lève une exception, il a échoué. C'est une grande victoire pour la clarté. Nous pouvons écrire du code qui traite clairement le cas normal de réussite et déplacer tout le code de gestion des erreurs dans des catchclauses. Il apparaît souvent que les détails sur la manière ou la raison pour laquelle une méthode a échoué ne sont pas importants pour l'appelant, de sorte que la même catchclause peut être utilisée pour gérer plusieurs types d'échec. Et il arrive souvent qu'une méthode n'a pas besoin d'exceptions de capture du tout , mais peut simplement leur permettre de se propager à son interlocuteur. Les exceptions dues à des bogues de programme sont dans cette dernière classe; peu de méthodes peuvent réagir de manière appropriée en cas de bogue.

Donc, ces méthodes qui reviennent null.

  • Une nullvaleur indique- t-elle un bogue dans votre code? Si c'est le cas, vous ne devriez pas du tout attraper l'exception. Et votre code ne doit pas essayer de se remettre en question. Écrivez simplement ce qui est clair et concis en supposant que cela fonctionnera. Une chaîne d'appels de méthode est-elle claire et concise? Alors utilisez-les simplement.
  • Une nullvaleur indique- t-elle une entrée invalide dans votre programme? Si c'est le cas, a NullPointerExceptionn'est pas une exception appropriée à lever, car il est conventionnellement réservé à l'indication de bogues. Vous souhaitez probablement lever une exception personnalisée dérivée de IllegalArgumentException(si vous voulez une exception non cochée ) ou IOException(si vous voulez une exception cochée). Votre programme doit-il fournir des messages d'erreur de syntaxe détaillés en cas d'entrée non valide? Si tel est le cas, vérifier chaque méthode pour une nullvaleur de retour, puis lancer une exception de diagnostic appropriée est la seule chose que vous pouvez faire. Si votre programme n'a pas besoin de fournir des diagnostics détaillés, il NullPointerExceptionest plus clair et plus concis de chaîner les appels de méthode, d'en attraper puis de lancer votre exception personnalisée.

L'une des réponses prétend que les appels de méthode en chaîne violent la loi de Déméter et sont donc mauvais. Cette affirmation est erronée.

  • En ce qui concerne la conception de programmes, il n'y a pas vraiment de règles absolues sur ce qui est bon et ce qui est mauvais. Il n'y a que des heuristiques: des règles qui sont correctes la plupart du temps (voire presque). Une partie de la compétence de la programmation est de savoir quand il est acceptable d'enfreindre ce genre de règles. Donc, une affirmation laconique selon laquelle "cela va à l'encontre de la règle X " n'est pas vraiment une réponse. Est-ce une des situations où la règle devrait être enfreinte?
  • La loi de Demeter est vraiment une règle sur la conception d'API ou d'interface de classe. Lors de la conception des classes, il est utile d'avoir une hiérarchie d'abstractions. Vous disposez de classes de bas niveau qui utilisent les primitives de langage pour effectuer directement des opérations et représenter des objets dans une abstraction de niveau supérieur à celui des primitives de langage. Vous avez des classes de niveau moyen qui délèguent aux classes de bas niveau et implémentent des opérations et des représentations à un niveau plus élevé que les classes de bas niveau. Vous avez des classes de haut niveau qui délèguent aux classes de niveau moyen et implémentent des opérations et des abstractions de niveau encore plus élevé. (J'ai parlé de seulement trois niveaux d'abstraction ici, mais d'autres sont possibles). Cela permet à votre code de s'exprimer en termes d'abstractions appropriées à chaque niveau, masquant ainsi la complexité. La justification de la loi de Demeterest que si vous avez une chaîne d'appels de méthode, cela suggère que vous avez une classe de haut niveau atteignant une classe de niveau moyen pour traiter directement les détails de bas niveau, et par conséquent que votre classe de niveau moyen n'a pas fourni d'opération abstraite de niveau moyen dont la classe de haut niveau a besoin. Mais il semble que ce ne soit pas la situation que vous avez ici: vous n'avez pas conçu les classes dans la chaîne d'appels de méthode, elles sont le résultat d'un code de sérialisation XML généré automatiquement (n'est-ce pas?), Et la chaîne d'appels n'est pas descendante à travers une hiérarchie d'abstraction parce que le XML désérialisé est au même niveau de la hiérarchie d'abstraction (n'est-ce pas?)?
Raedwald
la source
3

Pour améliorer la lisibilité, vous pouvez utiliser plusieurs variables, comme

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
la source
C'est beaucoup moins lisible à mon avis. Il jette dans la méthode tout un tas de logique de vérification de null qui est complètement hors de propos de la logique réelle de la méthode.
Developer102938
2

N'attrape pas NullPointerException. Vous ne savez pas d'où cela vient (je sais que ce n'est pas probable dans votre cas mais peut-être que quelque chose d'autre l'a jeté) et c'est lent. Vous souhaitez accéder au champ spécifié et pour cela, tous les autres champs ne doivent pas être nuls. C'est une raison valable parfaite pour vérifier chaque champ. Je le vérifierais probablement dans un si et puis créerais une méthode pour la lisibilité. Comme d'autres l'ont souligné, le retour de -1 est déjà très ancien mais je ne sais pas si vous avez une raison ou non (par exemple, parler à un autre système).

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

Edit: Il est discutable s'il désobéit à la loi de Demeter puisque le WsObject n'est probablement qu'une structure de données (consultez https://stackoverflow.com/a/26021695/1528880 ).

DerM
la source
2

Si vous ne souhaitez pas refactoriser le code et que vous pouvez utiliser Java 8, il est possible d'utiliser des références de méthode.

Une simple démo d'abord (excusez les classes internes statiques)

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

Production

241
4

nul
2

L'interface Gettern'est qu'une interface fonctionnelle, vous pouvez utiliser n'importe quel équivalent.
GetterResultclass, les accesseurs supprimés pour plus de clarté, contiennent le résultat de la chaîne getter, le cas échéant, ou l'index du dernier getter appelé.

La méthode getterChainest un morceau de code simple et standard, qui peut être généré automatiquement (ou manuellement si nécessaire).
J'ai structuré le code pour que le bloc répétitif soit évident.


Ce n'est pas une solution parfaite car vous devez toujours définir une surcharge de getterChainpar nombre de getters.

Je refactoriserais le code à la place, mais si vous ne pouvez pas et que vous vous trouvez en utilisant souvent de longues chaînes getter, vous pouvez souvent envisager de créer une classe avec des surcharges qui prennent de 2 à, disons, 10 getters.

Margaret Bloom
la source
2

Comme d'autres l'ont dit, le respect de la loi de Déméter fait définitivement partie de la solution. Une autre partie, dans la mesure du possible, consiste à modifier ces méthodes chaînées afin qu'elles ne puissent pas revenir null. Vous pouvez éviter de retourner nullen retournant à la place un objet vide String, vide Collectionou un autre objet factice qui signifie ou fait tout ce que l'appelant ferait null.

Kevin Krumwiede
la source
2

Je voudrais ajouter une réponse qui se concentre sur la signification de l'erreur . L'exception nulle en elle-même ne fournit aucune erreur complète de signification. Je vous conseille donc d'éviter de traiter directement avec eux.

Il y a des milliers de cas où votre code peut mal tourner: impossible de se connecter à la base de données, exception IO, erreur réseau ... Si vous les traitez un par un (comme le contrôle nul ici), ce serait trop compliqué.

Dans le code:

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

Même lorsque vous savez quel champ est nul, vous n'avez aucune idée de ce qui ne va pas. Peut-être que Bar est nul, mais est-il attendu? Ou est-ce une erreur de données? Pensez aux personnes qui lisent votre code

Comme dans la réponse de xenteros, je proposerais d'utiliser une exception personnalisée non cochée . Par exemple, dans cette situation: Foo peut être nul (données valides), mais Bar et Baz ne doivent jamais être nulles (données non valides)

Le code peut être réécrit:

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
la source
Les exceptions non cochées indiquent que le programmeur utilise mal l'API. Les problèmes externes tels que «impossible de se connecter à la base de données, exception IO, erreur réseau» doivent être indiqués par des exceptions cochées.
Kevin Krumwiede
Cela dépend vraiment du besoin de l'appelant. Aide aux exceptions vérifiée car elle vous oblige à traiter l'erreur Cependant, dans d'autres cas, cela n'est pas nécessaire et peut polluer le code. Par exemple, vous avez une IOException dans votre couche de données, allez-vous la lancer dans la couche de présentation? Cela signifierait que vous devez attraper l'exception et relancer chaque appelant. Je préfère envelopper l'IOException par une BusinessException personnalisée, avec un message pertinent, et la laisser apparaître dans le stacktrace, jusqu'à ce qu'un filtre global l'attrape et affiche le message à l'utilisateur.
Hoàng Long
Les appelants n'ont pas à attraper et à relancer les exceptions vérifiées, il suffit de les déclarer comme levées.
Kevin Krumwiede
@KevinKrumwiede: vous avez raison, il suffit de déclarer l'exception à lever. Nous devons encore déclarer cependant. Edit: En y jetant un deuxième coup d'oeil, il y a pas mal de débats sur les utilisations d'exceptions cochées et non cochées (par exemple: programmers.stackexchange.com/questions/121328/… ).
Hoàng Long
2

NullPointerException est une exception d'exécution, il n'est donc généralement pas recommandé de l'attraper, mais de l'éviter.

Vous devrez intercepter l'exception là où vous voulez appeler la méthode (ou elle se propagera dans la pile). Néanmoins, si dans votre cas vous pouvez continuer à travailler avec ce résultat avec la valeur -1 et que vous êtes sûr qu'il ne se propagera pas parce que vous n'utilisez aucun des "morceaux" qui peuvent être nuls, alors il me semble juste de attrape ça

Éditer:

Je suis d'accord avec la réponse ultérieure de @xenteros, il vaudra mieux lancer votre propre exception au lieu de renvoyer -1, vous pouvez l'appeler InvalidXMLExceptionpar exemple.

SCouto
la source
3
Que voulez-vous dire par "peu importe si vous l'attrapez, il peut se propager à d'autres parties du code"?
Hulk
Si la valeur null est dans cette phrase wsObject.getFoo () Et dans les parties ultérieures du code, vous exécutez à nouveau cette requête ou utilisez wsObject.getFoo (). GetBar () () (par exemple), il lèvera à nouveau une NullPointerException.
SCouto
C'est un libellé inhabituel pour "Vous devrez attraper l'exception partout où vous voulez appeler la méthode (ou elle se propagera dans la pile)." Si je comprends bien. Je suis d'accord avec cela (et cela peut être un problème), je trouve simplement que le libellé prête à confusion.
Hulk
Je vais le réparer, désolé, l'anglais n'est pas ma langue maternelle, alors cela peut arriver parfois :) Merci
SCouto
2

Je suis ce post depuis hier.

J'ai commenté / voté les commentaires qui disent, attraper NPE est mauvais. Voici pourquoi je fais cela.

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

Production

3,216

0,002

Je vois un gagnant clair ici. Avoir des chèques if est bien trop moins cher que d'attraper une exception. J'ai vu cette façon de faire Java-8. Considérant que 70% des applications actuelles fonctionnent toujours sur Java-7, j'ajoute cette réponse.

Conclusion Pour toutes les applications critiques, la gestion des NPE est coûteuse.

Nouvel utilisateur
la source
Trois secondes supplémentaires sur un million de demandes dans le pire des cas sont mesurables, mais ce serait rarement un facteur décisif, même dans les «applications critiques». Il existe des systèmes où l'ajout de 3,2 microsecondes à une requête est un gros problème, et si vous avez un tel système, réfléchissez bien aux exceptions. Mais appeler un service Web et désérialiser sa sortie, conformément à la question d'origine, prend probablement beaucoup plus de temps que cela, et s'inquiéter des performances de la gestion des exceptions est hors de propos.
Jeroen Mostert
@JeroenMostert: 3 secondes par chèque / million. Ainsi, le nombre de chèques augmentera le coût
NewUser
Vrai. Même avec cela, je le considérerais toujours comme un cas de «profil d'abord». Vous auriez besoin de plus de 300 chèques en une seule demande avant que la demande ne prenne une milliseconde supplémentaire. Les considérations de conception pèseraient sur mon âme bien plus tôt que cela.
Jeroen Mostert
@JeroenMostert: :) D'accord! Je voudrais laisser au programmeur le soin de donner le résultat et le laisser prendre un appel!
NewUser
1

Si l'efficacité est un problème, l'option «catch» doit être envisagée. Si 'catch' ne peut pas être utilisé car il se propagerait (comme mentionné par 'SCouto') alors utilisez des variables locales pour éviter plusieurs appels aux méthodes getFoo(), getBar()et getBaz().

JEP
la source
1

Cela vaut la peine d'envisager de créer votre propre exception. Appelons cela MyOperationFailedException. Vous pouvez le lancer à la place en renvoyant une valeur. Le résultat sera le même - vous quitterez la fonction, mais vous ne retournerez pas la valeur codée en dur -1 qui est l'anti-pattern Java. En Java, nous utilisons des exceptions.

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

ÉDITER:

D'après la discussion dans les commentaires, permettez-moi d'ajouter quelque chose à mes réflexions précédentes. Dans ce code, il y a deux possibilités. L'une est que vous acceptez null et l'autre est que c'est une erreur.

S'il s'agit d'une erreur et que cela se produit, vous pouvez déboguer votre code en utilisant d'autres structures à des fins de débogage lorsque les points d'arrêt ne suffisent pas.

Si c'est acceptable, vous ne vous souciez pas de l'endroit où ce null est apparu. Si vous le faites, vous ne devriez certainement pas enchaîner ces demandes.

xenteros
la source
2
Ne pensez-vous pas que supprimer l'exception est une mauvaise idée? En temps réel, si on perd la trace d'une exception, sa vraie douleur au fond pour savoir ce qui se passe! Je suggérerais toujours de ne pas utiliser le chaînage. Le deuxième problème que je vois est le suivant: ce code ne peut pas bénéficier à un moment donné, lequel du résultat était nul.
NewUser
Non, votre exception peut avoir un message qui indiquerait certainement l'endroit où elle a été lancée. Je suis d'accord que le chaînage n'est pas la meilleure solution :)
xenteros
3
Non, cela dirait simplement le numéro de ligne. Ainsi, n'importe lequel des appels de la chaîne peut conduire à une exception.
NewUser
"Si c'est une erreur et qu'elle se produit, vous pouvez déboguer votre code" - pas en production. Je préfère de loin savoir CE QUI a échoué quand tout ce que j'ai est un journal que d'essayer de deviner ce qui est arrivé à son échec. Avec ce conseil (et ce code), tout ce que vous savez vraiment, c'est que l'une des 4 choses était nulle, mais pas laquelle ni pourquoi.
VLAZ
1

La méthode que vous avez est longue, mais très lisible. Si j'étais un nouveau développeur venant dans votre base de code, je pourrais voir ce que vous faisiez assez rapidement. La plupart des autres réponses (y compris la capture de l'exception) ne semblent pas rendre les choses plus lisibles et certaines le rendent moins lisible à mon avis.

Étant donné que vous n'avez probablement pas de contrôle sur la source générée et en supposant que vous avez vraiment besoin d'accéder à quelques champs profondément imbriqués ici et là, je recommanderais d'encapsuler chaque accès profondément imbriqué avec une méthode.

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

Si vous vous retrouvez à écrire beaucoup de ces méthodes ou si vous êtes tenté de rendre ces méthodes statiques publiques, je créerais un modèle d'objet séparé, imbriqué comme vous le souhaitez, avec uniquement les champs qui vous intéressent, et convertirais à partir du Web services du modèle objet à votre modèle objet.

Lorsque vous communiquez avec un service Web distant, il est très courant d'avoir un «domaine distant» et un «domaine d'application» et de basculer entre les deux. Le domaine distant est souvent limité par le protocole Web (par exemple, vous ne pouvez pas envoyer de méthodes d'assistance dans un service RESTful pur et les modèles d'objet profondément imbriqués sont courants pour éviter plusieurs appels d'API) et donc pas idéal pour une utilisation directe dans votre client.

Par exemple:

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

}
Rythme
la source
1
return wsObject.getFooBarBazInt();

en appliquant la loi de Déméter,

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
la source
0

Donner une réponse qui semble différente de toutes les autres.

Je vous recommande de vérifier NULLdans ifs.

Raison:

Nous ne devons pas laisser une seule chance à notre programme de s'écraser. NullPointer est généré par le système. Le comportement des exceptions générées par le système ne peut pas être prédit . Vous ne devez pas laisser votre programme entre les mains de System si vous avez déjà un moyen de le gérer vous-même. Et mettez le mécanisme de traitement d'exception pour la sécurité supplémentaire. !!

Pour rendre votre code facile à lire, essayez ceci pour vérifier les conditions:

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

ÉDITER :

Ici , vous devez stocker ces valeurs wsObject.getFoo(), wsObject.getFoo().getBar(), wsObject.getFoo().getBar().getBaz()dans certaines variables. Je ne le fais pas parce que je ne connais pas les types de retour de ces fonctions.

Toute suggestion sera appréciée..!!

Janki Gadhiya
la source
Avez-vous considéré getFoo () comme une opération très chronophage? Vous devez stocker les valeurs renvoyées dans des variables, mais c'est un gaspillage de mémoire. Votre méthode est parfaite pour la programmation en C.
xenteros
mais il est parfois préférable d'avoir 1 milliseconde de retard, puis le programme se bloque @xenteros .. !!
Janki Gadhiya
getFoo () peut obtenir une valeur d'un serveur situé sur un autre continent. Cela peut durer à tout moment: mins / heures ...
xenteros
wsObjectcontiendra la valeur renvoyée par Webservice .. !! Le service sera déjà appelé et wsObjectobtiendra une longue XMLdonnée en réponse au service Web .. !! Il n'y a donc rien de tel qu'un serveur situé sur un autre continent car getFoo()c'est juste un élément obtenant une méthode getter et non un appel Webservice .. !! @xenteros
Janki Gadhiya
1
Eh bien, d'après les noms des getters, je suppose qu'ils renvoient des objets Foo, Bar et Baz: P Envisagez également de supprimer la double sécurité mentionnée de votre réponse. Je ne pense pas que cela apporte une valeur réelle à part la pollution du code. Avec des variables locales saines et une vérification des valeurs nulles, nous avons fait plus qu'assez pour garantir l'exactitude du code. Si une exception peut se produire, elle doit être considérée comme telle.
Marius K.
0

J'ai écrit une classe appelée Snagqui vous permet de définir un chemin pour naviguer dans un arbre d'objets. Voici un exemple de son utilisation:

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

Cela signifie que l'instance ENGINE_NAMEappellerait effectivementCar?.getEngine()?.getName() l'instance qui lui a été transmise, et retournerait nullsi une référence retournait null:

final String name =  ENGINE_NAME.get(firstCar);

Il n'est pas publié sur Maven mais si quelqu'un trouve cela utile, c'est ici (sans garantie bien sûr!)

C'est un peu basique mais cela semble faire le travail. De toute évidence, il est plus obsolète avec les versions plus récentes de Java et d'autres langages JVM qui prennent en charge la navigation sécurisée ou Optional.

Riches
la source