Style de constructeur Java : vérifier que les paramètres ne sont pas nuls

Question

Quelles sont les meilleures pratiques si vous avez une classe qui accepte certains paramètres mais qu'aucun d'entre eux n'est autorisé à être un paramètre de la classe ? null ?

Le point suivant est évident, mais l'exception est un peu moins précise :

public class SomeClass
{
     public SomeClass(Object one, Object two)
     {
        if (one == null || two == null)
        {
            throw new IllegalArgumentException("Parameters can't be null");
        }
        //...
     }
}

Ici, les exceptions permettent de savoir quel paramètre est nul, mais le constructeur est maintenant assez laid :

public class SomeClass
{
     public SomeClass(Object one, Object two)
     {
        if (one == null)
        {
            throw new IllegalArgumentException("one can't be null");
        }           
        if (two == null)
        {
            throw new IllegalArgumentException("two can't be null");
        }
        //...
  }

Ici, le constructeur est plus soigné, mais le code du constructeur n'est plus vraiment dans le constructeur :

public class SomeClass
{
     public SomeClass(Object one, Object two)
     {
        setOne(one);
        setTwo(two);
     }

     public void setOne(Object one)
     {
        if (one == null)
        {
            throw new IllegalArgumentException("one can't be null");
        }           
        //...
     }

     public void setTwo(Object two)
     {
        if (two == null)
        {
            throw new IllegalArgumentException("two can't be null");
        }
        //...
     }
  }

Lequel de ces styles est le meilleur ?

Ou existe-t-il une alternative plus largement acceptée ?

Answer 1

Le deuxième ou le troisième.

Parce qu'il indique à l'utilisateur de votre API ce qui n'a pas fonctionné exactement.

Pour moins de verbosité, utilisez Validate.notNull(obj, message) de commons-lang. Ainsi, votre constructeur ressemblera à

public SomeClass(Object one, Object two) {
    Validate.notNull(one, "one can't be null");
    Validate.notNull(two, "two can't be null");
    ...
}

Il est également possible de placer le contrôle dans le setter, avec le même commentaire sur la verbosité. Si vos setters ont également pour rôle de préserver la cohérence de l'objet, vous pouvez également opter pour la troisième solution.

Answer 2

Vous pouvez utiliser l'une des nombreuses bibliothèques conçues pour faciliter la vérification des conditions préalables. De nombreux codes en Google Goyave utilise com.google.common.base.Preconditions

Méthodes statiques simples à appeler au début de vos propres méthodes pour vérifier que les arguments et l'état sont corrects. Cela permet à des constructions telles que

 if (count <= 0) {
   throw new IllegalArgumentException("must be positive: " + count);
 }

pour être remplacée par l'option plus compacte

 checkArgument(count > 0, "must be positive: %s", count);

Il a checkNotNull c'est-à-dire largement utilisé dans le cadre de Guava . Vous pouvez alors écrire :

 import static com.google.common.base.Preconditions.checkNotNull;
 //...

 public SomeClass(Object one, Object two) {
     this.one = checkNotNull(one);
     this.two = checkNotNull(two, "two can't be null!");
     //...
 }

La plupart des méthodes sont surchargées pour ne prendre aucun message d'erreur, un message d'erreur fixe ou un message d'erreur modélisé avec des varargs.

---

Sur IllegalArgumentException vs NullPointerException

Alors que votre code original lance IllegalArgumentException sur null arguments, les arguments de Guava Preconditions.checkNotNull lancers NullPointerException au lieu de cela.

Voici une citation de Java efficace 2ème édition : Point 60 : Favoriser l'utilisation des exceptions standard :

On peut soutenir que toutes les invocations de méthodes erronées se résument à un argument illégal ou à un état illégal, mais d'autres exceptions sont généralement utilisées pour certains types d'arguments et d'états illégaux. Si un appelant passe null dans certains paramètres pour lesquels les valeurs nulles sont interdites, la convention veut que NullPointerException plutôt que IllegalArgumentException .

A NullPointerException n'est pas réservé à l'accès aux membres d'un fichier null référence ; il est assez courant de les lancer lorsqu'un argument est null alors qu'il s'agit d'une valeur illégale.

System.out.println("some string".split(null));
// throws NullPointerException

Answer 3

J'aurais une méthode utilitaire :

 public static <T> T checkNull(String message, T object) {
     if(object == null) {
       throw new NullPointerException(message);
     }
     return object;
  }

Je voudrais qu'il renvoie l'objet pour que vous puissiez l'utiliser dans des affectations comme celle-ci :

 public Constructor(Object param) {
     this.param = checkNull("Param not allowed to be null", param);
 }

EDIT : En ce qui concerne les suggestions d'utiliser une bibliothèque tierce, Google Preconditions en particulier fait ce qui précède encore mieux que mon code. Cependant, si c'est la seule raison d'inclure la bibliothèque dans votre projet, j'hésiterais. La méthode est trop simple.

Answer 4

Outre les réponses données ci-dessus, qui sont toutes valables et raisonnables, je pense qu'il est bon de souligner que la vérification de la nullité n'est peut-être pas nécessairement une "bonne pratique". (En supposant que des lecteurs autres que l'OP puissent prendre la question comme dogmatique)

Extrait du blog de Misko Hevery sur la testabilité : Affirmer ou ne pas affirmer

Answer 5

Une alternative à la levée d'une exception non vérifiée serait l'utilisation de la fonction assert . Sinon, je lancerais des exceptions vérifiées pour que l'appelant sache que le constructeur ne fonctionnera pas avec des valeurs illégales.

La différence entre vos deux premières solutions - avez-vous besoin d'un message d'erreur détaillé, avez-vous besoin de savoir quel paramètre a échoué ou est-il suffisant de savoir que l'instance n'a pas pu être créée en raison d'arguments illégaux ?

Notez que les deuxième et troisième exemples ne peuvent pas signaler correctement que les deux paramètres sont nuls.

BTW - Je vote pour une variante de (1) :

if (one == null || two == null) {
    throw new IllegalArgumentException(
      String.format("Parameters can't be null: one=%s, two=%s", one, two));
}