Question

Supposons que vous utilisiez ces deux méthodes:

Numéro 1:

void AddPerson(Person person)
{
  // Validate person
  if(person.Name != null && IsValidDate(person.BirthDate)
    DB.AddPersonToDatabase(person);
}

Numéro 2:

void AddPerson(string name, DateTime birthDate)
{
  Person p = new Person(name, birthDate);
  DB.AddPersonToDatabase(person);
}

Laquelle des deux méthodes est la meilleure? Je sais que le premier est plus correct en ce qui concerne OO, mais je pense que le second est plus lisible, et vous n'avez pas à vous assurer que l'objet est valide, car les paramètres le permettent. Je n'aime simplement pas avoir à valider les objets partout où je les passe en paramètres. Y a-t-il d'autres approches?

EDIT: Merci pour toutes les réponses. Pour clarifier, valider dans le constructeur et une méthode IsValid est bien sûr une bonne approche, mais dans mon code, l’état valide de la personne dépend souvent du contexte et peut varier d’une méthode à l’autre. Cela pourrait bien sûr être un signe de mauvaise conception.

Le code n'est qu'un exemple pour décrire le problème.

Était-ce utile?

La solution

Le premier ne devrait pas avoir à valider person.Name et person.BirthDate - ils devraient être validés automatiquement par le constructeur Person . En d’autres termes, si vous êtes une personne, vous devez savoir qu’elle est valide.

D'autre part, vous devrez vérifier que personne n'est pas une référence null.

Cela vaut parfois la peine d'utiliser des méthodes simples comme la deuxième version pour ne pas avoir à appeler explicitement le constructeur aussi souvent. Il convient généralement d'appeler simplement le constructeur Person , puis de déléguer le travail au premier formulaire.

Autres conseils

Le premier a l'avantage de vous permettre de changer la définition de la personne sans casser le code existant. Seule une recompilation est nécessaire. Vous pouvez penser que le second est plus lisible, mais le premier est plus facile à maintenir, votre choix.

Une autre option serait:

void AddPerson(Person person)
{  // Validate person  
   if(person.IsValid)
   {
     DB.AddPersonToDatabase(person);
   }
}

En supposant que cette personne ne se valide pas lorsqu'elle est construite. Ce qui, dans certains cas, est viable si l’objet peut être un état non valide tant qu’il est transitoire.

Je préfère le premier (transmettre un objet) car il réduit le couplage de l'API à l'objet. Si vous modifiez l'objet Personne , par exemple ajoutez une nouvelle propriété, telle que Surnom , qui n'était pas nécessaire auparavant. Dans le premier cas, vous n'avez pas besoin de modifier l'API publique, alors que dans le second, vous devez modifier la méthode ou ajoutez une nouvelle surcharge.

Je conviens que cela dépend entièrement du contexte, il n'y a pas de règles absolues pour cela. À mon avis, il serait absurde d'avoir des méthodes comme celles-ci:

person.SetBirthDate(Person person)
person.ResetPassword(Person person)

Mais dans ce cas, je préfère le premier, parce que, comme l'a dit Greg Beech, la méthode ne sait pas (nécessairement) quoi que ce soit sur l'objet de domaine.

Au fait, envisagez une surcharge (DRY - ne vous répétez pas):

void AddPerson(Person person)
{
  if(person.Name != null && IsValidDate(person.BirthDate)
    DB.AddPersonToDatabase(person);
}

void AddPerson(string name, DateTime birthDate)
{
  Person p = new Person(name, birthDate);
  this.AddPerson(p);
}

Il est certainement préférable de passer un objet Personne plutôt qu’un ensemble de types primitifs en tant que paramètres. Comparez les deux méthodes suivantes


public static void Withdrawal(Account account, decimal amount)
{
    DB.UpdateBalance(account.AccountNumber, amount);
}

public static void Withdraw(int accountNumber, decimal amount)
{
    DB.UpdateBalance(accountNumber, amount);
}

Les deux méthodes semblent presque identiques, mais la seconde est dangereuse. Un int peut venir de n'importe où, alors vous êtes foutu si vous écrivez ceci:

private void CloseTransaction(Transaction tran)
{
    BankAccounts.Withdrawal(tran.Account.RoutingNumber, tran.Amount);
        // logic error: meant to pass Account.AccountNumber instead of Account.RoutingNumber
}

C’est le pire type d’erreur car il ne génère pas d’erreur de compilation ni d’exception au moment de l’exécution. Si vous écrivez suffisamment bien, vous risquez de détecter cette erreur dans vos tests automatisés, mais il est facile de rater ce bogue et de vous cacher pendant des mois sans que vous ne le découvriez.

J'ai travaillé pour une société qui a écrit le logiciel de banque et nous avons rencontré un bogue de ce type en production. Cela ne s'est produit qu'au cours d'un type spécifique de transfert de coffre, et cela n'a été découvert que lorsqu'une de nos banques a constaté que son solde GL était réduit de quelques 100 dollars à chaque fois qu'il exécutait son processus de fin de mois. La banque a suspecté un vol d’employé pendant des mois, mais ce n’est que par un examen minutieux du code que le problème a été attribué à un bogue dans notre logiciel.

J'irais la plupart du temps pour le premier.

Le second casse la signature pour chaque changement de personne

Cela dépend du contexte.

Si toutes vos méthodes d'appel traitent avec des objets Personne, la première solution est la bonne.

Toutefois, si certaines de vos méthodes d'appel traitent du nom et de la date de naissance, et que d'autres traitent des objets Personne, le second exemple est la solution correcte.

Je créerais simplement des surcharges pour les deux. Surtout en considérant qu'ils peuvent être créés automatiquement.

Je suppose que les méthodes ne font pas partie du type Personne . Dans cet esprit, j’ai le sentiment que les deux d’entre eux ont un peu trop de connaissances sur un autre type ( Personne ) imo. La première version ne devrait pas avoir à valider la création d'une instance de personne valide. Si cela relève de la responsabilité de l'appelant, chaque appelant doit le faire. C'est fragile.

La deuxième version a une dépendance forte à un autre Type en raison du fait qu’elle crée une instance de ce type.

Je préférerais certainement la première version, mais je déplacerais la partie validation de cette partie du code.

Je pense que Jon a bien réussi. Person doit être responsable de la création d’une personne valide dans son constructeur.

À propos de la responsabilité qui crée ou non l'objet Personne (que ce soit la méthode AddPerson ou son appelant), lisez

.

http://en.wikipedia.org/wiki/GRASP_%28Object_Oriented_Design % 29 # créateur

Il s’agit de questions de responsabilité en POO. Dans votre cas spécifique, si AddPerson encapsule l'appel vers une interface de base de données, je ne suis pas tout à fait sûr à ce sujet. Cela dépend de l'utilisation de cet objet Personne en dehors de ce contexte. Si c'est uniquement dans le but de contenir les données à ajouter à la base de données, les créer dans la méthode AddPerson est probablement une bonne idée, car cela dissocie l'utilisateur de votre classe de la nécessité de connaître la classe Person. .

Une meilleure encapsulation si vous utilisez des objets; c'est ce qu'ils sont pour. Je pense que les gens ont des problèmes quand ils utilisent trop les primitives.

Il ne devrait pas être possible de créer une personne invalide. Le constructeur doit vérifier les paramètres valides et émettre IllegalArgumentException ou échouer avec une assertion s’ils ne sont pas valides. C’est en cela que consiste la programmation par contrat.

Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top