Je suis tombé sur la pièce de code suivante lors d'une revue de code.
Mon intuition me dit que cela ne suit pas la bonne oop.
Je pense que la méthode LoadObject devrait Renvoie un nouvel objet ToNobject, au lieu de modifier celui-ci transmis. Bien que je ne puisse pas vraiment trouver une explication appropriée de la raison pour laquelle c'est mieux.
est ma solution meilleure? et si oui pourquoi? Spécifiquement, quels principes ou normes OOP sont cassés dans l'exemple de code donné (le cas échéant)?
6 Réponses :
Il n'y a rien de mal à la manière dont le code est écrit puisque vous ne modifiez qu'une propriété sur ToOObject.
Cependant, il serait tout aussi correct de créer quelque téléphone dans LoadSomeObject et de le renvoyer.
À ce stade, les deux choix sont aussi corrects.
Je ne suis pas un oo gourou, alors prenez-les avec un grain de sel.
Les objets doivent se gérer eux-mêmes / définir leur comportement aussi loin que possible. La justification derrière cela est évidente si vous vous dirigez jamais. Je peux très bien être la meilleure décision de conception pour déplacer les détails de L'état mutable est parfaitement parfaite dans n'importe quel code impératif (y compris le code OO), c'est une "fonctionnalité" du noyau de ces paradigmes. L'OTOH, l'état immuable a des avantages indéfectables (je suppose que nous avons quelques questions sur ce sujet ici, sinon demandez à tout avocat FP) et que des objets immuables ne sont pas particulièrement non OO.
Edit: Vous pouvez également passer au lecteur au constructeur de SomeObject. loadsomeObject à la mise en œuvre de ToOObject, mais c'est difficile à discuter pour un tel exemple général.
Toute raison particulière ceci a été accepté sur la réponse de Russjudge avec 4 fois plus de susvotes?
L'une ou l'autre manière est parfaitement acceptable, mais des considérations sur ce que vous souhaitez faire avec l'objet retourné viennent de jouer lorsque vous décidez qui convient à votre situation particulière. Immutabilité I.e Création d'une nouvelle instance à chaque opération est la façon dont les chaînes de .NET sont manipulées.
Une méthode de copie devra évidemment renvoyer une copie. Où comme méthode qui fonctionne sur un objet singleton, par exemple, qui est détenue par une référence globale ne conviendrait pas à renvoyer un nouvel objet car les modifications peuvent être perdues.
Si loadsomeObject va retourner un nouveau TooObject , vous pouvez modifier le nom de la méthode pour éviter toute confusion. Peut-être à NewandloadSomeObject ?
Je suis d'accord avec votre intuition, que cela se sent un peu éteint dans la plupart des cas. Je conviendrais que le retour Quelque chose est meilleur, comme il suffit de préciser que quelque chose est modifié. Voir une valeur retournée utilisée, il est immédiatement clair ce qui se passe.
Je pense que cela se sent un peu mieux encore, mais (dans la plupart des cas): P>
public void Load(SqlDataReader reader)
{
this.Id = reader.GetGuid(0);
}
Il n'y a pas de concept universel OO, qui est en conflit avec le code comme celui-là. Mais plus tard, vous constaterez que c'est difficile à collecter et à comprendre le moyen de manipuler des instances de quelque manière que vous ne suivez pas certains principes de conception. Peut-être que le moyen le plus simple de démarreur est de séparer deux types de procédures principales:
Une bonne idée ici, si vous souhaitez séparer ToObject Manipulate Logic consiste à créer un type de type SomeObjectBuilder avec P>
public void loadFromReader( SqlDataReader reader )
On dirait que vous pouvez muter
sqldatreader aussi.