Validation du paramètre d'entrée dans l'appelant: duplication de code?

Où est le meilleur endroit pour valider les paramètres d'entrée de la fonction: dans l'appelant ou dans la fonction elle-même?

Comme je voudrais améliorer mon style de codage, j'essaie de trouver les meilleures pratiques ou quelques règles pour ce problème. Quand et quoi de mieux.

Dans mes projets précédents, nous avions l'habitude de vérifier et de traiter chaque paramètre d'entrée à l'intérieur de la fonction (par exemple s'il n'est pas nul). Maintenant, j'ai lu ici dans certaines réponses et aussi dans le livre du programmeur pragmatique, que la validation du paramètre d'entrée est la responsabilité de l'appelant.

Cela signifie donc que je dois valider les paramètres d'entrée avant d'appeler la fonction. Partout la fonction est appelée. Et cela soulève une question: ne crée-t-il pas une duplication de la condition de vérification partout où la fonction est appelée?

Je ne m'intéresse pas seulement aux conditions nulles, mais à la validation des variables d'entrée (valeur négative pour sqrtfonctionner, diviser par zéro, mauvaise combinaison d'état et de code postal, ou autre)

Existe-t-il des règles pour décider où vérifier la condition d'entrée?

Je pense à quelques arguments:

• lorsque le traitement d'une variable invalide peut varier, il est bon de la valider côté appelant (par exemple, sqrt()fonction - dans certains cas, je peux vouloir travailler avec un nombre complexe, donc je traite la condition dans l'appelant)
• lorsque la condition de vérification est la même pour chaque appelant, il est préférable de la vérifier à l'intérieur de la fonction, pour éviter les doublons
• la validation du paramètre d'entrée dans l'appelant n'a lieu qu'une seule avant d'appeler de nombreuses fonctions avec ce paramètre. Par conséquent, la validation d'un paramètre dans chaque fonction n'est pas efficace
• la bonne solution dépend du cas particulier

J'espère que cette question n'est pas en double d'aucune autre, j'ai cherché ce problème et j'ai trouvé des questions similaires mais ils ne mentionnent pas exactement ce cas.

Ça dépend. Le choix de l'emplacement de la validation doit être basé sur la description et la force du contrat impliqué (ou documenté) par la méthode. La validation est un bon moyen de renforcer l'adhésion à un contrat spécifique. Si pour une raison quelconque, la méthode a un contrat très strict, alors oui, c'est à vous de vérifier avant d'appeler.

Il s'agit d'un concept particulièrement important lorsque vous créez une méthode publique , car vous annoncez essentiellement qu'une certaine méthode effectue une opération. Il vaut mieux faire ce que vous dites!

Prenons l'exemple de la méthode suivante:

public void DeletePerson(Person p)
{            
    _database.Delete(p);
}

Quel est le contrat sous-entendu DeletePerson? Le programmeur peut seulement supposer que s'il Persony en a un, il sera supprimé. Cependant, nous savons que ce n'est pas toujours vrai. Et si pc'est une nullvaleur? Et si pn'existe pas dans la base de données? Et si la base de données est déconnectée? Par conséquent, DeletePerson ne semble pas remplir correctement son contrat. Parfois, il supprime une personne, et parfois il lève une NullReferenceException, ou une DatabaseNotConnectedException, ou parfois il ne fait rien (comme si la personne est déjà supprimée).

Les API comme celle-ci sont notoirement difficiles à utiliser, car lorsque vous appelez cette «boîte noire» d'une méthode, toutes sortes de choses terribles peuvent se produire.

Voici quelques façons d'améliorer le contrat:

• Ajoutez la validation et ajoutez une exception au contrat. Cela renforce le contrat , mais nécessite que l'appelant effectue la validation. La différence, cependant, c'est que maintenant ils connaissent leurs besoins. Dans ce cas, je communique cela avec un commentaire XML C #, mais vous pouvez à la place ajouter un throws(Java), utiliser un Assertou utiliser un outil de contrat comme Code Contracts.

  ///<exception>ArgumentNullException</exception>
  ///<exception>ArgumentException</exception>
  public void DeletePerson(Person p)
  {            
      if(p == null)
          throw new ArgumentNullException("p");
      if(!_database.Contains(p))
          throw new ArgumentException("The Person specified is not in the database.");
  
      _database.Delete(p);
  }
  

  _{Note latérale: L'argument contre ce style est souvent qu'il provoque une pré-validation excessive par tout le code appelant, mais selon mon expérience, ce n'est souvent pas le cas. Imaginez un scénario dans lequel vous essayez de supprimer une personne nulle. Comment est-ce arrivé? D'où venait la Personne nulle? S'il s'agit d'une interface utilisateur, par exemple, pourquoi la touche Supprimer a-t-elle été gérée s'il n'y a pas de sélection actuelle? S'il a déjà été supprimé, ne devrait-il pas déjà avoir été supprimé de l'écran? Évidemment, il y a des exceptions à cela, mais au fur et à mesure qu'un projet se développe, vous remercierez souvent du code comme celui-ci pour empêcher les bogues de pénétrer profondément dans le système.}

• Ajoutez la validation et le code de manière défensive. Cela rend le contrat plus lâche , car maintenant cette méthode fait plus que simplement supprimer la personne. J'ai changé le nom de la méthode pour refléter cela, mais peut ne pas être nécessaire si vous êtes cohérent dans votre API. Cette approche a ses avantages et ses inconvénients. Le pro étant que vous pouvez maintenant appeler en TryDeletePersonpassant toutes sortes d'entrées invalides et ne vous inquiétez jamais des exceptions. L'inconvénient, bien sûr, est que les utilisateurs de votre code appelleront probablement trop cette méthode, ou cela pourrait rendre le débogage difficile dans les cas où p est nul. Cela pourrait être considéré comme une légère violation du principe de responsabilité unique , alors gardez cet esprit si une guerre des flammes éclate.

  public void TryDeletePerson(Person p)
  {            
      if(p == null || !_database.Contains(p))
          return;
  
      _database.Delete(p);
  }
  

• Combinez les approches. Parfois, vous voulez un peu des deux, où vous voulez que les appelants externes suivent les règles de près (pour les forcer à coder de manière responsable), mais vous voulez que votre code privé soit flexible.

  ///<exception>ArgumentNullException</exception>
  ///<exception>ArgumentException</exception>
  public void DeletePerson(Person p)
  {            
      if(p == null)
          throw new ArgumentNullException("p");
      if(!_database.Contains(p))
          throw new ArgumentException("The Person specified is not in the database.");
  
      TryDeletePerson(p);
  }
  
  internal void TryDeletePerson(Person p)
  {            
      if(p == null || !_database.Contains(p))
          return;
  
      _database.Delete(p);
  }
  

D'après mon expérience, se concentrer sur les contrats que vous avez impliqués plutôt que sur une règle stricte fonctionne mieux. Le codage défensif semble mieux fonctionner dans les cas où il est difficile ou difficile pour l'appelant de déterminer si une opération est valide. Les contrats stricts semblent mieux fonctionner lorsque vous vous attendez à ce que l'appelant ne fasse des appels de méthode que lorsqu'ils ont vraiment, vraiment du sens.

C'est une question de convention, de documentation et de cas d'utilisation.

Toutes les fonctions ne sont pas égales. Toutes les exigences ne sont pas égales. Toutes les validations ne sont pas égales.

Par exemple, si votre projet Java essaie d'éviter les pointeurs nuls dans la mesure du possible (voir les recommandations de style Guava , par exemple), validez-vous toujours chaque argument de fonction pour vous assurer qu'il n'est pas nul? Ce n'est probablement pas nécessaire, mais il est probable que vous le fassiez toujours, pour faciliter la recherche de bogues. Mais vous pouvez utiliser une assertion où vous avez précédemment levé une NullPointerException.

Et si le projet est en C ++? La convention / tradition en C ++ consiste à documenter les conditions préalables, mais seulement à les vérifier (le cas échéant) dans les versions de débogage.

Dans les deux cas, vous avez une condition préalable documentée sur votre fonction: aucun argument ne peut être nul. Vous pouvez à la place étendre le domaine de la fonction pour inclure des valeurs nulles avec un comportement défini, par exemple "si un argument est nul, lève une exception". Bien sûr, c'est encore mon héritage C ++ qui parle ici - en Java, c'est assez commun pour documenter les conditions préalables de cette façon.

Mais toutes les conditions préalables ne peuvent même pas être raisonnablement vérifiées. Par exemple, un algorithme de recherche binaire a la condition préalable que la séquence à rechercher doit être triée. Mais vérifier qu'il en est bien ainsi est une opération O (N), ce faisant, à chaque appel, cela vainc un peu le point d'utiliser un algorithme O (log (N)) en premier lieu. Si vous programmez de manière défensive, vous pouvez effectuer des vérifications moindres (par exemple en vérifiant que pour chaque partition que vous recherchez, les valeurs de début, de milieu et de fin sont triées), mais cela ne détecte pas toutes les erreurs. En règle générale, vous devrez simplement vous fier à la condition préalable remplie.

Le seul endroit réel où vous avez besoin de vérifications explicites est aux limites. Contribution externe à votre projet? Valider, valider, valider. Une zone grise correspond aux limites de l'API. Cela dépend vraiment de combien vous voulez faire confiance au code client, des dommages causés par une entrée non valide et de l'aide que vous souhaitez apporter à la recherche de bogues. Toute limite de privilège doit être considérée comme externe, bien sûr - les appels système, par exemple, s'exécutent dans un contexte de privilèges élevés et doivent donc être très prudents à valider. Une telle validation doit bien entendu être interne à l'appel système.

La validation des paramètres doit être la préoccupation de la fonction appelée. La fonction doit savoir ce qui est considéré comme une entrée valide et ce qui ne l'est pas. Les appelants peuvent ne pas le savoir, surtout lorsqu'ils ne savent pas comment la fonction est implémentée en interne. La fonction doit être censée gérer toute combinaison de valeurs de paramètres des appelants.

Étant donné que la fonction est responsable de la validation des paramètres, vous pouvez écrire des tests unitaires par rapport à cette fonction pour vous assurer qu'elle se comporte comme prévu avec des valeurs de paramètre valides et non valides.

Au sein de la fonction elle-même. Si la fonction est utilisée plusieurs fois, vous ne voudriez pas vérifier le paramètre pour chaque appel de fonction.

De plus, si la fonction est mise à jour de manière à affecter la validation du paramètre, vous devez rechercher chaque occurrence de la validation de l'appelant pour les mettre à jour. Ce n'est pas beau :-).

Vous pouvez vous référer à la clause de garde

Mise à jour

Voir ma réponse pour chaque scénario que vous avez fourni.

• lorsque le traitement d'une variable invalide peut varier, il est bon de la valider côté appelant (par exemple, sqrt()fonction - dans certains cas, je peux vouloir travailler avec un nombre complexe, donc je traite la condition dans l'appelant)

  Répondre

  La majorité des langages de programmation prend en charge les nombres entiers et réels par défaut, et non les nombres complexes, d'où leur implémentation sqrtaccepte uniquement les nombres non négatifs. Le seul cas où vous avez une sqrtfonction qui renvoie un nombre complexe est lorsque vous utilisez un langage de programmation orienté vers les mathématiques, comme Mathematica

  De plus, sqrtpour la plupart des langages de programmation est déjà implémenté, donc vous ne pouvez pas le modifier, et si vous essayez de remplacer l'implémentation (voir patch de singe), alors vos collaborateurs seront tout à fait choqués de savoir pourquoi sqrtaccepte soudainement les nombres négatifs.

  Si vous en vouliez un, vous pouvez l'enrouler autour de votre sqrtfonction personnalisée qui gère le nombre négatif et renvoie un nombre complexe.

• lorsque la condition de vérification est la même pour chaque appelant, il est préférable de la vérifier à l'intérieur de la fonction, pour éviter les doublons

  Répondre

  Oui, c'est une bonne pratique pour éviter de disperser la validation des paramètres dans votre code.

• la validation du paramètre d'entrée dans l'appelant n'a lieu qu'une seule avant d'appeler de nombreuses fonctions avec ce paramètre. Par conséquent, la validation d'un paramètre dans chaque fonction n'est pas efficace

  Répondre

  Ce sera bien si l'appelant est une fonction, vous ne trouvez pas?

  Si les fonctions de l'appelant sont utilisées par un autre appelant, qu'est-ce qui vous empêche de valider le paramètre dans les fonctions appelées par l'appelant?

• la bonne solution dépend du cas particulier

  Répondre

  Visez un code maintenable. Le déplacement de la validation de vos paramètres garantit une source de vérité sur ce que la fonction peut accepter ou non.

Une fonction doit indiquer ses conditions préalables et postérieures.
Les conditions préalables sont les conditions qui doivent être remplies par l'appelant avant de pouvoir utiliser correctement la fonction et peuvent (et le font souvent) inclure la validité des paramètres d'entrée.
Les post-conditions sont les promesses que la fonction fait à ses appelants.

Lorsque la validité des paramètres d'une fonction fait partie des conditions préalables, il est de la responsabilité de l'appelant de s'assurer que ces paramètres sont valides. Mais cela ne signifie pas que chaque appelant doit vérifier explicitement chaque paramètre avant l'appel. Dans la plupart des cas, aucun test explicite n'est nécessaire car la logique interne et les conditions préalables de l'appelant garantissent déjà la validité des paramètres.

Par mesure de sécurité contre les erreurs de programmation (bugs), vous pouvez vérifier que les paramètres transmis à une fonction remplissent réellement les conditions préalables énoncées. Comme ces tests peuvent être coûteux, c'est une bonne idée de pouvoir les désactiver pour les versions. Si ces tests échouent, alors le programme doit être arrêté, car il a prouvé qu'il a rencontré un bogue.

Bien qu'à première vue la vérification de l'appelant semble inviter à la duplication de code, c'est en fait l'inverse. La vérification dans l'appelé entraîne une duplication de code et de nombreux travaux inutiles.
Pensez-y, à quelle fréquence passez-vous des paramètres à travers plusieurs couches de fonctions, en n'apportant que de petites modifications à certaines d'entre elles en cours de route. Si vous appliquez systématiquement la méthode check-in-callee , chacune de ces fonctions intermédiaires devra refaire la vérification pour chacun des paramètres.
Et imaginez maintenant que l'un de ces paramètres est censé être une liste triée.
Avec la vérification dans l'appelant, seule la première fonction devrait s'assurer que cette liste est vraiment triée. Tous les autres savent que la liste est déjà triée (comme c'est ce qu'ils ont déclaré dans leur condition préalable) et peuvent la transmettre sans autre vérification.

Le plus souvent, vous ne pouvez pas savoir qui, quand et comment appellera la fonction que vous avez écrite. Il vaut mieux supposer le pire: votre fonction sera appelée avec des paramètres invalides. Vous devriez donc certainement couvrir cela.

Néanmoins, si la langue que vous utilisez prend en charge les exceptions, vous pouvez ne pas vérifier certaines erreurs et être sûr qu'une exception sera levée, mais dans ce cas, vous devez être sûr de décrire le cas dans la documentation (vous devez avoir de la documentation). L'exception donnera à l'appelant suffisamment d'informations sur ce qui s'est passé et attirera également l'attention sur les arguments non valides.