Dois-je valider la valeur de retour d'un appel de méthode même si je sais que la méthode ne peut pas renvoyer une entrée incorrecte?

30

Je me demande si je devrais me défendre contre la valeur de retour d'un appel de méthode en validant qu'il répond à mes attentes même si je sais que la méthode que j'appelle répondra à ces attentes.

DONNÉ

User getUser(Int id)
{
    User temp = new User(id);
    temp.setName("John");

    return temp;
}

DEVRAIS-JE

void myMethod()
{
    User user = getUser(1234);
    System.out.println(user.getName());
}

OU

void myMethod()
{
    User user = getUser(1234);

    // Validating
    Preconditions.checkNotNull(user, "User can not be null.");
    Preconditions.checkNotNull(user.getName(), "User's name can not be null.");

    System.out.println(user.getName());
}

Je pose cette question au niveau conceptuel. Si je connais le fonctionnement interne de la méthode que j'appelle. Soit parce que je l'ai écrit ou que je l'ai inspecté. Et la logique des valeurs possibles qu'elle renvoie répond à mes conditions préalables. Est-il «préférable» ou «plus approprié» d'ignorer la validation, ou devrais-je tout de même me défendre contre de mauvaises valeurs avant de poursuivre avec la méthode que j'implémente actuellement, même si elle doit toujours réussir.


Ma conclusion de toutes les réponses (n'hésitez pas à venir à la vôtre):

Affirmez quand
  • La méthode a montré un mauvais comportement dans le passé
  • La méthode provient d'une source non fiable
  • La méthode est utilisée depuis d'autres endroits et n'indique pas explicitement ses post-conditions
N'affirmez pas quand:
  • La méthode est proche de la vôtre (voir la réponse choisie pour plus de détails)
  • La méthode définit explicitement son contrat avec quelque chose comme un bon document, une sécurité de type, un test unitaire ou une vérification post-condition
  • Les performances sont essentielles (dans ce cas, une assertion de mode de débogage pourrait fonctionner comme une approche hybride)
Didier A.
la source
1
Visez la couverture du code% 100!
Jared Burrows
9
Pourquoi vous concentrez-vous sur un seul problème ( Null)? C'est probablement aussi problématique si le nom est ""(pas nul mais la chaîne vide) ou "N/A". Soit vous pouvez faire confiance au résultat, soit vous devez être paranoïaque de manière appropriée.
MSalters
3
Dans notre base de code, nous avons un schéma de nommage: getFoo () promet de ne pas retourner null, et getFooIfPresent () peut retourner null.
pjc50
D'où vient votre présomption de bon sens du code? Supposez-vous que la valeur sera toujours non nulle parce qu'elle est validée à l'entrée ou à cause de la structure par laquelle la valeur est récupérée? Et, plus important encore, testez-vous tous les scénarios de cas de bord possibles? Y compris la possibilité de script malveillant?
Zibbobz

Réponses:

42

Cela dépend de la probabilité que getUser et myMethod changent, et plus important encore, de leur probabilité de changer indépendamment l'un de l'autre .

Si vous savez d'une manière ou d'une autre que getUser ne changera jamais, jamais, jamais à l'avenir, alors oui, c'est une perte de temps de le valider, autant que de perdre du temps à valider qui ia une valeur de 3 immédiatement après une i = 3;déclaration. En réalité, vous ne le savez pas. Mais vous savez certaines choses:

  • Ces deux fonctions "cohabitent-elles"? En d'autres termes, ont-ils les mêmes personnes qui les gèrent, font-ils partie du même fichier et sont-ils donc susceptibles de rester «synchronisés» les uns avec les autres? Dans ce cas, il est probablement exagéré d'ajouter des vérifications de validation, car cela signifie simplement plus de code qui doit changer (et potentiellement générer des bogues) chaque fois que les deux fonctions changent ou sont refactorisées en un nombre différent de fonctions.

  • La fonction getUser fait-elle partie d'une API documentée avec un contrat spécifique, et myMethod est-il simplement un client de ladite API dans une autre base de code? Si c'est le cas, vous pouvez lire cette documentation pour savoir si vous devez valider les valeurs de retour (ou pré-valider les paramètres d'entrée!) Ou s'il est vraiment sûr de suivre aveuglément le chemin heureux. Si la documentation ne le dit pas clairement, demandez au responsable de corriger cela.

  • Enfin, si cette fonction particulière a soudainement et de manière inattendue changé son comportement dans le passé, d'une manière qui a cassé votre code, vous avez le droit d'être paranoïaque à son sujet. Les bugs ont tendance à se regrouper.

Notez que tout ce qui précède s'applique même si vous êtes l'auteur original des deux fonctions. Nous ne savons pas si ces deux fonctions devraient "vivre ensemble" pour le reste de leur vie, ou si elles se sépareront lentement en modules séparés, ou si vous vous êtes tiré une balle dans le pied avec un bug dans les anciennes versions. versions de getUser. Mais vous pouvez probablement faire une supposition assez décente.

Ixrec
la source
2
Aussi: Si vous deviez changer getUserplus tard, afin qu'il puisse retourner null, la vérification explicite vous donnerait-elle des informations que vous ne pourriez pas obtenir de "NullPointerException" et du numéro de ligne?
user253751
Il semble qu'il y ait deux choses que vous pouvez valider: (1) Que la méthode fonctionne correctement, comme dans, elle n'a pas de bugs. (2) Que la méthode respecte votre contrat. Je peux voir la validation pour que # 1 soit exagéré. Vous devriez plutôt avoir des tests faisant # 1. C'est pourquoi je ne validerais pas i = 3 ;. Ma question porte donc sur # 2. J'aime votre réponse pour cela, mais en même temps, c'est une réponse à votre jugement. Voyez-vous un autre problème découlant de la validation explicite de vos contrats, autre que de perdre un peu de temps à rédiger l'assertion?
Didier A.
Le seul autre "problème" est que si votre interprétation du contrat est fausse, ou devient fausse, alors vous devez aller modifier toute la validation. Mais cela s'applique également à tous les autres codes.
Ixrec
1
Je ne peux pas aller contre la foule. Cette réponse est certainement la plus correcte pour couvrir le sujet de ma question. Je voudrais ajouter qu'en lisant toutes les réponses, en particulier @MikeNakis, je pense maintenant que vous devriez affirmer vos conditions préalables, au moins en mode débogage, lorsque vous avez l'un des deux derniers points de puce d'Ixrec. Si vous êtes confronté au premier point, il est probablement exagéré d'affirmer vos conditions préalables. Enfin, étant donné un contrat explicite, comme un bon document, une sécurité de type, un test unitaire ou un contrôle post-condition, vous ne devriez pas affirmer vos conditions préalables, ce serait exagéré.
Didier A.
22

La réponse d'Ixrec est bonne, mais je vais adopter une approche différente car je pense que cela vaut la peine d'être considéré. Aux fins de cette discussion, je parlerai d'affirmations, puisque c'est le nom sous lequel votre nom Preconditions.checkNotNull()est traditionnellement connu.

Ce que je voudrais suggérer, c'est que les programmeurs surestiment souvent le degré auquel ils sont certains que quelque chose se comportera d'une certaine manière, et sous-estimeront les conséquences de ne pas se comporter de cette façon. De plus, les programmeurs ne réalisent souvent pas la puissance des assertions comme documentation. Par la suite, d'après mon expérience, les programmeurs affirment les choses beaucoup moins souvent qu'ils ne le devraient.

Ma devise est:

La question que vous devriez toujours vous poser n'est pas "devrais-je affirmer cela?" mais "y a-t-il quelque chose que j'ai oublié d'affirmer?"

Naturellement, si vous êtes absolument sûr que quelque chose se comporte d'une certaine manière, vous vous abstiendrez d'affirmer qu'il s'est effectivement comporté de cette façon, et c'est pour la plupart raisonnable. Il n'est pas vraiment possible d'écrire un logiciel si vous ne pouvez faire confiance à rien.

Mais il existe même des exceptions à cette règle. Curieusement, pour prendre l'exemple d'Ixrec, il existe un type de situation où il est parfaitement souhaitable de suivre int i = 3;avec une assertion qui se isitue effectivement dans une certaine fourchette. C'est la situation où iest susceptible d'être modifié par quelqu'un qui essaie divers scénarios de simulation, et le code qui suit repose sur iune valeur dans une certaine plage, et il n'est pas immédiatement évident en regardant rapidement le code suivant ce la plage acceptable est. Donc, cela int i = 3; assert i >= 0 && i < 5;peut sembler à première vue insensé, mais si vous y réfléchissez, cela vous indique que vous pouvez jouer avec i, et cela vous indique également la plage dans laquelle vous devez rester. Une assertion est le meilleur type de documentation, caril est appliqué par la machine , c'est donc une garantie parfaite, et il est refactorisé avec le code , il reste donc toujours pertinent.

Votre getUser(int id)exemple n'est pas un parent conceptuel très éloigné de l' assign-constant-and-assert-in-rangeexemple. Vous voyez, par définition, des bugs se produisent lorsque les choses présentent un comportement différent de celui que nous attendions. Certes, nous ne pouvons pas tout affirmer, donc parfois il y aura un débogage. Mais c'est un fait bien établi dans l'industrie que plus nous détectons rapidement une erreur, moins cela coûte cher. Les questions que vous devez poser ne sont pas seulement de savoir à quel point vous êtes sûr de getUser()ne jamais retourner null (et de ne jamais renvoyer un utilisateur avec un nom nul), mais aussi, quels types de mauvaises choses se produiront avec le reste du code si c'est le cas dans un jour, retournez quelque chose d'inattendu, et combien c'est facile en regardant rapidement le reste du code pour savoir précisément ce qu'on attend de lui getUser().

Si le comportement inattendu entraînerait la corruption de votre base de données, vous devriez peut-être affirmer même si vous êtes sûr à 101% que cela ne se produira pas. Si un nullnom d'utilisateur provoque une erreur cryptique étrange introuvable, des milliers de lignes plus bas et des milliards de cycles d'horloge plus tard, il est peut-être préférable d'échouer le plus tôt possible.

Donc, même si je ne suis pas en désaccord avec la réponse d'Ixrec, je vous suggère de considérer sérieusement le fait que les affirmations ne coûtent rien, donc il n'y a vraiment rien à perdre, seulement à gagner, en les utilisant généreusement.

Mike Nakis
la source
2
Oui. Affirmez-le. Je suis venu ici pour donner plus ou moins cette réponse. ++
RubberDuck
2
@SteveJessop Je corrigerais cela ... && sureness < 1).
Mike Nakis
2
@MikeNakis: c'est toujours un cauchemar lors de l'écriture de tests unitaires, la valeur de retour qui est autorisée par l'interface mais impossible à générer réellement à partir des entrées ;-) Quoi qu'il en soit, lorsque vous êtes sûr à 101% que vous avez raison, vous avez définitivement tort !
Steve Jessop
2
+1 pour avoir souligné des choses que j'avais complètement oublié de mentionner dans ma réponse.
Ixrec
1
@DavidZ C'est exact, ma question supposait une validation au moment de l'exécution. Mais dire que vous ne faites pas confiance à la méthode dans le débogage et que vous lui faites confiance dans prod est un bon point de vue sur le sujet. Une affirmation de style C pourrait donc être un bon moyen de faire face à cette situation.
Didier A.
8

Un non catégorique.

Un appelant ne doit jamais vérifier si la fonction qu'il appelle respecte son contrat. La raison est simple: il existe potentiellement de très nombreux appelants et il est peu pratique et sans doute même erroné d'un point de vue conceptuel pour chaque appelant de dupliquer la logique de vérification des résultats d'autres fonctions.

Si des contrôles doivent être effectués, chaque fonction doit vérifier ses propres post-conditions. Les post-conditions vous donnent l'assurance que vous avez correctement mis en œuvre la fonction et les utilisateurs pourront se fier au contrat de votre fonction.

Si une certaine fonction n'est pas censée retourner null, alors cela devrait être documenté et faire partie du contrat de cette fonction. C'est le but de ces contrats: offrir aux utilisateurs des invariants sur lesquels ils peuvent compter pour qu'ils rencontrent moins d'obstacles lors de l'écriture de leurs propres fonctions.

Paul
la source
Je suis d'accord avec le sentiment général, mais il y a des circonstances où il est vraiment logique de valider les valeurs de retour. Par exemple si vous appelez un Web Service externe, vous souhaitez parfois valider au moins le format de la réponse (xsd, etc ...)
AK_
On dirait que vous préconisez un style de conception par contrat plutôt qu'un style de programmation défensive. Je pense que c'est une excellente réponse. Si la méthode avait un contrat strict, je pouvais lui faire confiance. Dans mon cas, ce n'est pas le cas, mais je pourrais le changer pour le faire. Dis que je ne pouvais pas? Diriez-vous que je dois faire confiance à la mise en œuvre? Même s'il n'indique pas explicitement dans son doc qu'il ne retourne pas null, ou a en fait un contrôle post-condition?
Didier A.
À mon avis, vous devriez utiliser votre meilleur jugement et pondérer différents aspects, tels que la confiance que vous accordez à l'auteur pour faire la bonne chose, la probabilité que l'interface de la fonction change et la rigueur de vos contraintes de temps. Cela étant dit, la plupart du temps, l'auteur d'une fonction a une idée assez claire de ce que devrait être le contrat de la fonction, même s'il ne le documente pas. Pour cette raison, je dis que vous devez d'abord faire confiance aux auteurs pour faire la bonne chose et ne devenir défensif que lorsque vous savez que la fonction n'est pas bien écrite.
Paul
Je trouve que faire confiance aux autres pour faire la bonne chose m'aide à faire le travail plus rapidement. Cela étant dit, le type d'applications que j'écris est facile à corriger en cas de problème. Quelqu'un qui travaille avec des logiciels plus critiques pour la sécurité pourrait penser que je suis trop indulgent et préférer une approche plus défensive.
Paul
5

Si null est une valeur de retour valide de la méthode getUser, votre code doit gérer cela avec un If, pas une exception - ce n'est pas un cas exceptionnel.

Si null n'est pas une valeur de retour valide de la méthode getUser, alors il y a un bogue dans getUser - la méthode getUser doit lever une exception ou sinon être corrigée.

Si la méthode getUser fait partie d'une bibliothèque externe ou ne peut pas être modifiée et que vous souhaitez que des exceptions soient levées dans le cas d'un retour nul, encapsulez la classe et la méthode pour effectuer les vérifications et lever l'exception afin que chaque instance de l'appel à getUser est cohérent dans votre code.

MatthewC
la source
Null n'est pas une valeur de retour valide de getUser. En inspectant getUser, j'ai vu que l'implémentation ne retournerait jamais null. Ma question est la suivante: dois-je faire confiance à mon inspection et ne pas avoir de chèque dans ma méthode, ou dois-je douter de mon inspection et avoir encore un chèque. Il est également possible que la méthode change à l'avenir, brisant ainsi mes attentes de ne pas retourner null.
Didier A.
Et que se passe-t-il si getUser ou user.getName changent pour lever des exceptions? Vous devriez avoir une vérification, mais pas dans le cadre de la méthode utilisant l'utilisateur. Enveloppez la méthode getUser - et même la classe User - pour appliquer vos contrats, si vous vous inquiétez des modifications futures de la rupture du code getUser. Créez également des tests unitaires pour vérifier vos hypothèses sur le comportement de la classe.
MatthewC
@didibus Pour paraphraser votre réponse ... Une émoticône souriante n'est pas une valeur de retour valide de getUser. En inspectant getUser, j'ai vu que l'implémentation ne retournerait jamais une émoticône souriante . Ma question est la suivante: dois-je faire confiance à mon inspection et ne pas avoir de chèque dans ma méthode, ou dois-je douter de mon inspection et avoir encore un chèque. Il est également possible que la méthode change à l'avenir, brisant mes attentes de ne pas retourner d' émoticône souriante . Il n'appartient pas à l'appelant de confirmer que la fonction appelée remplit son contrat.
Vince O'Sullivan
@ VinceO'Sullivan Il semble que le "contrat" ​​soit au centre du problème. Et que ma question concerne les contrats implicites, par opposition à explicites. Une méthode qui renvoie int Je n'affirmerai pas que je n'obtiens pas de chaîne. Mais, si je ne veux que des pouces positifs, le devrais-je? C'est seulement implicitement que la méthode ne retourne pas un entier négatif, car je l'ai inspecté. Il ne ressort pas clairement de la signature ou de la documentation que ce n'est pas le cas.
Didier A.
1
Peu importe que le contrat soit implicite ou explicite. Ce n'est pas le travail du code appelant de valider que la méthode appelée renvoie des données valides lorsqu'elle reçoit des données valides. Dans votre exemple, vous savez que les inits négatifs ne sont pas des réponses incorrectes mais savez-vous si les int positifs sont corrects? De combien de tests avez-vous vraiment besoin pour prouver que la méthode fonctionne? Le seul cours correct est de supposer que la méthode est correcte.
Vince O'Sullivan
5

Je dirais que la prémisse de votre question est fausse: pourquoi utiliser une référence nulle pour commencer?

Le modèle d'objet null est un moyen de renvoyer des objets qui ne font essentiellement rien: au lieu de renvoyer null pour votre utilisateur, renvoyez un objet qui représente «aucun utilisateur».

Cet utilisateur serait inactif, aurait un nom vide et d'autres valeurs non nulles indiquant «rien ici». Le principal avantage est que vous n'avez pas besoin de salir votre programme pour annuler les vérifications, la journalisation, etc. vous utilisez simplement vos objets.

Pourquoi un algorithme devrait-il se soucier d'une valeur nulle? Les étapes doivent être les mêmes. Il est tout aussi facile et beaucoup plus clair d'avoir un objet nul qui renvoie zéro, une chaîne vide, etc.

Voici un exemple de vérification de code pour null:

User u = getUser();
String displayName = "";
if (u != null) {
  displayName = u.getName();
}
return displayName;

Voici un exemple de code utilisant un objet nul:

return getUser().getName();

Qu'est-ce qui est plus clair? Je crois que le deuxième est.


Bien que la question soit balisée , il convient de noter que le modèle d'objet nul est vraiment utile en C ++ lors de l'utilisation de références. Les références Java ressemblent davantage à des pointeurs C ++ et autorisent null. Les références C ++ ne peuvent pas tenir nullptr. Si l'on souhaite avoir quelque chose d'analogue à une référence null en C ++, le modèle d'objet null est le seul moyen que je connaisse pour y parvenir.

Communauté
la source
Null n'est que l'exemple de ce que j'affirme. Il se pourrait très bien que j'aie besoin d'un nom non vide. Ma question s'appliquerait toujours, valideriez-vous cela? Ou ce pourrait être quelque chose qui renvoie un int, et je ne peux pas avoir un int négatif, ou il doit être dans une plage donnée. Ne pensez pas que c'est un problème nul, disons.
Didier A.
1
Pourquoi une méthode retournerait-elle une valeur incorrecte? Le seul endroit où cela doit être vérifié est à la limite de votre application: les interfaces avec l'utilisateur et avec d'autres systèmes. Sinon, c'est pourquoi nous avons des exceptions.
Imaginez que j'avais: int i = getCount; flotteur x = 100 / i; Si je sais que getCount ne renvoie pas 0, car j'ai écrit la méthode ou l'ai inspectée, dois-je ignorer la vérification de 0?
Didier A.
@didibus: vous pouvez ignorer la vérification si des getCount() documents garantissent qu'il ne retourne pas maintenant 0, et ne le feront pas à l'avenir, sauf dans les cas où quelqu'un décide d'apporter une modification de rupture et cherche à mettre à jour tout ce qui l'appelle pour y faire face. la nouvelle interface. Voir ce que le code fait actuellement n'est pas utile, mais si vous allez inspecter le code, le code à inspecter est le test unitaire getCount(). S'ils le testent pour ne pas retourner 0, alors vous savez que tout changement futur pour retourner 0 sera considéré comme un changement de rupture car les tests échoueront.
Steve Jessop
Il n'est pas clair pour moi que les objets nuls sont meilleurs que les nuls. Un canard nul peut charlatan, mais ce n'est pas un canard - en fait, sémantiquement, c'est l'antithèse d'un canard. Une fois que vous avez introduit un objet nul, tout code utilisant cette classe peut devoir vérifier si l'un de ses objets est l'objet nul - par exemple, vous avez quelque chose de nul dans un ensemble, combien de choses avez-vous? Cela me semble être un moyen de différer l'échec et de masquer les erreurs. L'article lié met en garde spécifiquement contre l'utilisation d'objets nuls «juste pour éviter les vérifications nulles et rendre le code plus lisible».
sdenham
1

En général, je dirais que cela dépend principalement des trois aspects suivants:

  1. robustesse: la méthode appelante peut-elle supporter la nullvaleur? Si une nullvaleur peut entraîner un RuntimeExceptionje recommanderais une vérification - à moins que la complexité soit faible et que les méthodes appelées / appelantes soient créées par le même auteur et nullne soient pas attendues (par exemple les deux privatedans le même package).

  2. responsabilité du code: si le développeur A est responsable de la getUser()méthode et que le développeur B l'utilise (par exemple dans le cadre d'une bibliothèque), je recommande fortement de valider sa valeur. Tout simplement parce que le développeur B peut ne pas être au courant d'un changement qui entraîne une nullvaleur de retour potentielle .

  3. complexité: plus la complexité globale du programme ou de l'environnement est élevée, plus je recommanderais de valider la valeur de retour. Même si vous êtes sûr que cela ne peut pas être nullaujourd'hui dans le contexte de la méthode d'appel, il se peut que vous deviez changer getUser()pour un autre cas d'utilisation. Une fois que certains mois ou années ont disparu et que des milliers de lignes de codes ont été ajoutées, cela peut être un piège.

De plus, je recommande de documenter les nullvaleurs de retour potentielles dans un commentaire JavaDoc. En raison de la mise en évidence des descriptions JavaDoc dans la plupart des IDE, cela peut être un avertissement utile pour quiconque l'utilise getUser().

/** Returns ....
  * @param: id id of the user
  * @return: a User instance related to the id;
  *          null, if no user with identifier id exists
 **/
User getUser(Int id)
{
    ...
}
André
la source
-1

Vous n'avez certainement pas à le faire.

Par exemple, si vous savez déjà qu'un retour ne peut jamais être nul - Pourquoi voudriez-vous ajouter un chèque nul? L'ajout d'une vérification nulle ne va rien casser, mais c'est simplement redondant. Et mieux vaut ne pas coder la logique redondante tant que vous pouvez l'éviter.

Yellen
la source
1
Je sais, mais seulement parce que j'ai inspecté ou écrit l'implémentation de l'autre méthode. Le contrat de la méthode ne dit pas explicitement qu'elle ne peut pas. Cela pourrait donc, par exemple, changer à l'avenir. Ou il pourrait avoir un bogue qui provoque le retour d'un null même s'il essayait de ne pas le faire.
Didier A.