Vérifiez les paramètres annotés avec @Nonnull pour null?

19

Nous avons commencé à utiliser FindBugs et à annoter nos paramètres de @Nonnullmanière appropriée, et cela fonctionne très bien pour signaler les bogues au début du cycle. Jusqu'à présent, nous avons continué à vérifier ces arguments pour nullutiliser les goyaves checkNotNull, mais je préférerais vérifier nulluniquement sur les bords - des endroits où la valeur peut entrer sans avoir été vérifiée null, par exemple, une demande SOAP.

// service layer accessible from outside
public Person createPerson(@CheckForNull String name) {
    return new Person(Preconditions.checkNotNull(name));
}

...

// internal constructor accessed only by the service layer
public Person(@Nonnull String name) {
    this.name = Preconditions.checkNotNull(name);  // remove this check?
}

Je comprends que @Nonnullcela ne bloque pas les nullvaleurs en soi.

Cependant, étant donné que FindBugs indiquera n'importe où une valeur est transférée d'un champ non marqué à un champ marqué @Nonnull, ne pouvons-nous pas en dépendre pour attraper ces cas (ce qu'il fait) sans avoir à vérifier ces valeurs nullpartout où ils sont transmis le système? Suis-je naïf de vouloir faire confiance à l'outil et d'éviter ces vérifications détaillées?

Bottom line: Bien qu'il semble sûr de supprimer la deuxième nullvérification ci-dessous, est-ce une mauvaise pratique?

Cette question est peut-être trop similaire à Devrait-on vérifier la nullité s'il ne s'attend pas à la nullité , mais je pose la question spécifiquement par rapport à l' @Nonnullannotation.

David Harkness
la source

Réponses:

6

Si vous ne faites pas confiance à FindBug, cela peut être une option pour utiliser une assertion. De cette façon, vous évitez les problèmes mentionnés par User MSalters mais vous avez toujours un contrôle. Bien sûr, cette approche peut ne pas être applicable si une telle approche à échec rapide n'est pas possible, c'est-à-dire que vous devez intercepter toute exception.

Et pour répondre davantage à la question de savoir s'il s'agit d'une mauvaise pratique. Eh bien, c'est discutable, mais je ne pense pas. Je considère cette annotation FindBug comme une spécification légère suivant le principe de la conception par contrat.

Dans la conception par contrat, vous établissez un contrat entre une méthode et son appelant. Le contrat se compose de conditions préalables que l'appelant accepte de remplir lors de l'appel de la méthode et d'une postcondition qui à son tour est remplie par la méthode après l'exécution si sa condition préalable est remplie.

L'un des avantages de cette méthode de développement est que vous pouvez éviter un style de programmation dit défensif (dans lequel vous vérifiez explicitement toutes les valeurs de paramètres invalides, etc.). Au lieu de cela, vous pouvez compter sur le contrat et éviter les contrôles redondants pour augmenter les performances et la lisibilité.

Les contrats peuvent être utilisés pour la vérification des assertions d'exécution qui suit le principe de l'échec d'abord mentionné ci-dessus dans lequel vous voulez que votre programme échoue si un contrat est rompu, vous donnant un indice pour identifier la source d'erreur. (Une précondition échouée signifie que l'appelant a fait quelque chose de mal, une postcondition échouée indique une erreur d'implémentation de la méthode donnée)

De plus, les contrats peuvent être utilisés pour l'analyse statique, qui tente de tirer des conclusions sur le code source sans l'exécuter réellement.

L'annotation @nonnull peut être considérée comme une condition préalable utilisée pour l'analyse statique par l'outil FindBugs. Si vous préférez une approche telle que la vérification des assertions à l'exécution, c'est-à-dire la première stratégie d'échec, vous devriez envisager d'utiliser les instructions d'assertion en tant que Java intégré. Ou peut-être pour s'adapter à une stratégie de spécification plus sophistiquée en utilisant un langage de spécification d'interface comportementale tel que le langage de modélisation Java (JML).

JML est une extension de Java dans laquelle la spécification est intégrée dans des commentaires Java spéciaux. Un avantage de cette approche est que vous pouvez utiliser des spécifications sophistiquées, même en utilisant une approche de développement dans laquelle vous ne vous fiez qu'aux spécifications plutôt qu'aux détails d'implémentation et utilisez différents outils qui utilisent la spécification, tels que les outils de vérification des assertions d'exécution mentionnés, génération automatisée de tests unitaires ou de documentation.

Si vous partagez mon point de vue selon lequel @nonnull est un contrat (léger), il serait courant de ne pas ajouter de vérifications supplémentaires car l'idée originale derrière ce principe est d'éviter la programmation défensive.

FabianB
la source
2
C'est exactement ce que j'utilise @Nonnull: comme spécification de contrat. J'ai supprimé les chèques défensifs derrière le contrat et jusqu'à présent, je n'ai eu aucun problème.
David Harkness
4

Eh bien, réfléchissez à pourquoi vous n'écrivez pas

this.name = Preconditions.checkNotNull(name);  // Might be necessary
that.name = Preconditions.checkNotNull(this.name);  // Who knows?

La programmation n'est pas de la magie noire. Les variables ne deviennent pas soudainement nulles. Si vous savez qu'une variable n'est pas Null et que vous savez qu'elle n'a pas été modifiée, ne la vérifiez pas.

Si vous le vérifiez, un programmeur à l'avenir (peut-être vous) passera beaucoup de temps à découvrir pourquoi ce contrôle est là. Le chèque doit être là pour une raison, après tout, alors qu'est-ce que vous négligez?

MSalters
la source
3
Le @Nonnullcontrat stipule que les appelants ne doivent pas passer nullau constructeur et FindBugs utilise une analyse statique pour localiser les appels qui pourraient autoriser - nullspécifiquement les appels qui transmettent des valeurs non marquées @Nonnulleux-mêmes. Mais un appelant est libre de passer nullet d'ignorer l'avertissement de FindBugs.
David Harkness
La programmation n'est idéalement pas de la magie noire. Malheureusement, les mises en garde et les surprises abondent, surtout lorsque vous travaillez avec une partie du code d'accès concurrentiel ♬ ~ ᕕ (ᐛ) ᕗ
Tim Harper