2009-10-30 3 views
3

J'ai l'habitude de toujours valider les setters de propriétés contre les mauvaises données, même s'il n'y a pas d'endroit dans mon programme qui pourrait raisonnablement entrer de mauvaises données. Ma personne ne veut pas que je lève des exceptions à moins que je puisse expliquer où elles se produiraient. Dois-je valider toutes les propriétés? Y a-t-il une norme à ce sujet?OOP Design Question - Valider les propriétés

Exemple:

public void setName(String newName){ 
    if (newName == null){ 
     throw new IllegalArgumentException("Name cannot be null"); 
    } 
    name = newName; 
} 

... 

//Only call to setName(String) 
t.setName("Jim"); 
+1

Il semble que votre personne ne reçoive pas de couverture de test unitaire pour ces branches de lancer de contrôle. Vous devriez leur dire d'arrêter d'être paresseux, et de couvrir ceux qui sont dans les tests, comme il se doit. –

+0

Tests unitaires? Nous n'avons pas besoin de tests unitaires ... (je sais, je sais, mais je ne peux pas tout changer ...) –

Répondre

4

Vous appliquez les conditions préalables de votre méthode, qui sont une partie importante de son contrat. Il n'y a rien de mal à le faire, et cela sert aussi de code auto-documenté (si je lis le code de votre méthode, je vois immédiatement ce que je ne devrais pas lui passer), mais assert s peut être préférable pour cela.

1

Vous faites ok! Que ce soit un setter ou une fonction - validez toujours et lancez une exception significative. vous ne savez jamais quand vous en aurez besoin, et vous ...

0

C'est un compromis. Il y a plus de code à écrire, réviser et maintenir, mais vous trouverez probablement des problèmes plus rapidement si un nom nul passe à travers.

Je penche pour l'avoir parce que finalement vous trouvez que vous en avez besoin. J'avais l'habitude d'avoir des classes utilitaires pour garder le code au minimum. Ainsi, au lieu de

if (name == null) { throw new ... 

vous pourriez avoir

Util.assertNotNull(name) 

Ensuite Java ajouté à la langue affirme et vous pouvez le faire plus directement. Et éteignez-le si vous le vouliez.

2

Personnellement, je préfère utiliser Asserts dans ces cas extrêmement improbables pour éviter d'avoir du mal à lire le code, mais pour préciser que des hypothèses sont faites dans les algorithmes de la fonction.

Mais, bien sûr, c'est un jugement qui doit être fait au cas par cas. Vous pouvez le voir (et je l'ai vu) devenir complètement incontrôlable - au point où une simple fonction est transformée en un enchevêtrement d'instructions if qui n'évaluent presque jamais à vrai.

0

C'est bien fait à mon avis. Pour les valeurs nulles, lancez IllegalArgumentException. Pour d'autres types de validations, vous devriez envisager d'utiliser une hiérarchie d'exceptions personnalisée liée à vos objets de domaine.

1

En général, je ne favorise pas cette pratique. Ce n'est pas que la validation performante soit mauvaise, mais plutôt, sur des choses comme les simples setters, cela tend à créer plus d'encombrement que de valeur à protéger contre les bugs. Je préfère utiliser des tests unitaires pour m'assurer qu'il n'y a pas de bugs.

0

Je ne connais pas de norme documentée qui dit «valider toutes les entrées utilisateur», mais c'est une très bonne idée. Dans la version actuelle du programme, il peut ne pas être possible d'accéder à cette propriété particulière avec des données non valides, mais cela ne l'empêchera pas de se produire à l'avenir. Toutes sortes de choses intéressantes se produisent pendant la maintenance. Et bon, vous ne savez jamais quand quelqu'un réutilisera la classe dans une autre application qui ne validera pas les données avant de la transmettre.

1

Eh bien, ça fait longtemps que cette question a été postée mais j'aimerais donner un point de vue différent sur ce sujet.En utilisant l'exemple spécifique que vous avez posté, à mon humble avis vous devriez faire la validation, mais d'une manière différente.

La clé de la validation de l'archivage réside dans la question elle-même. Pensez-y: vous avez affaire à des noms, pas à des chaînes. Une chaîne est un nom lorsqu'elle n'est pas nulle. Nous pouvons également penser à des caractéristiques supplémentaires qui font d'une chaîne un nom: est ne peut pas être vide ni contenir d'espaces. Supposons que vous ayez besoin d'ajouter ces règles de validation: si vous respectez votre approche, vous finirez par encombrer votre setter comme l'a dit @SingleShot.

En outre, que feriez-vous si plus d'un objet de domaine a un setter setName? Même si vous utilisez des classes auxiliaires comme le fait @dave, le code sera toujours dupliqué: les appels aux instances auxiliaires. Maintenant, réfléchissez un instant: et si tous les arguments que vous pouviez jamais recevoir dans la méthode setName étaient valides? Aucune validation ne serait nécessaire à coup sûr. Je peux sembler trop optimiste, mais cela peut être fait. Rappelez-vous que vous avez affaire à des noms, alors pourquoi ne pas modéliser le concept d'un nom? Voici une vanille, la mise en œuvre factice pour montrer l'idée:

public class Name 

    public static Name From(String value) { 
    if (string.IsNullOrEmpty(value)) throw new ... 
    if (value.contains(' ')) throw new ... 

    return new Name(value); 
    } 

    private Name(string value) { 
    this.value = value; 
    } 

    // other Name stuff goes here... 
} 

Parce que la validation se produit au moment de la création, vous ne pouvez obtenir des instances de noms valides. Il n'y a aucun moyen de créer une instance Name à partir d'une chaîne "invalide". Non seulement le code de validation a été centralisé, mais des exceptions sont également lancées dans un contexte qui a un sens (la création d'une instance Name).

Vous pouvez lire sur les grands principes de conception dans "Design Principles Behind Patagonia" de Hernan Wilkinson (l'exemple de nom en est tiré). Soyez sûr de vérifier le ESUG 2010 Video et le presentation slides

Enfin, je pense que vous pourriez trouver l'article de Jim Shore "Fail Fast" intéressant.