Quel est le nom de cette mauvaise pratique / anti-pattern?

J’essaie d’expliquer à mon équipe pourquoi c’est une mauvaise pratique et je cherche une référence anti-modèle pour m’aider dans mon explication. Ceci est une très grande application d’entreprise, voici donc un exemple simple pour illustrer ce qui a été mis en œuvre:

public void ControlStuff() { var listOfThings = LoadThings(); var listOfThingsThatSupportX = new ssortingng[] {"ThingA","ThingB", "ThingC"}; foreach (var thing in listOfThings) { if(listOfThingsThatSupportX.Contains(thing.Name)) { DoSomething(); } } } 

Je suggère que nous ajoutions une propriété à la classe de base ‘Things’ pour nous dire si elle prend en charge X, car la sous-classe Thing devra implémenter la fonctionnalité en question. Quelque chose comme ça:

 public void ControlStuff() { var listOfThings = LoadThings(); foreach (var thing in listOfThings) { if (thing.SupportsX) { DoSomething(); } } } class ThingBase { public virtual bool SupportsX { get { return false; } } } class ThingA : ThingBase { public override bool SupportsX { get { return true; } } } class ThingB : ThingBase { } 

Donc, il est évident que la première approche est une mauvaise pratique, mais comment ça s’appelle? En outre, existe-t-il un modèle mieux adapté à ce problème que celui que je propose?

Normalement, une meilleure approche (IMHO) serait d’utiliser des interfaces plutôt que l’inheritance

il suffit ensuite de vérifier si l’object a implémenté l’interface ou non.

Je pense que le nom anti-pattern est codé en dur 🙂

La présence éventuelle de ThingBase.supportsX dépend au moins en partie de ce qu’est X Dans de rares cas, cette connaissance peut être uniquement dans ControlStuff() .

Plus généralement, X peut être un ensemble de choses, auquel cas ThingBase peut avoir besoin d’exposer ses capacités à l’aide de ThingBase.supports(ThingBaseProperty) ou d’autres.

IMO le principe de conception fondamental en jeu ici est l’encapsulation. Dans votre solution proposée, vous avez encapsulé la logique à l’intérieur de la classe Thing, où, comme dans le code d’origine, la logique est diffusée dans les appelants.

Il viole également le principe Open-Closed, car si vous souhaitez append de nouvelles sous-classes qui prennent en charge X, vous devez maintenant modifier n’importe quel élément contenant cette liste codée en dur. Avec votre solution, il vous suffit d’append la nouvelle classe, de remplacer la méthode et vous avez terminé.

Ne sais pas sur un nom (le doute existe), mais pensez à chaque “Chose” comme une voiture – certaines voitures ont un régulateur de vitesse et d’autres pas.

Maintenant, vous avez une flotte de voitures que vous gérez et que vous voulez savoir qui ont un régulateur de vitesse.

Utiliser la première approche est comme trouver une liste de tous les modèles de voiture qui ont un régulateur de vitesse, puis aller en voiture et chercher chacun dans cette liste – si cela signifie que la voiture a un régulateur de vitesse, sinon il n’a pas. Lourd, non?

Utiliser la deuxième approche signifie que chaque voiture qui a un régulateur de vitesse est munie d’un autocollant indiquant «J’ai un régulateur de vitesse» et que vous devez simplement chercher cet autocollant sans avoir recours à une source externe pour vous apporter des informations.

Explication pas très technique, mais simple et directe.

Il existe une situation tout à fait raisonnable où cette pratique de codage a du sens. Ce n’est peut-être pas une question de savoir quelles sont les choses qui supportent réellement X (où bien sûr une interface sur chaque chose serait meilleure), mais plutôt quelles choses qui supportent X sont celles que vous voulez activer . Le libellé de ce que vous voyez est alors simplement une configuration , actuellement codée en dur , et l’amélioration consiste à le déplacer éventuellement vers un fichier de configuration ou autre. Avant de persuader votre équipe de la modifier, je vérifierais que ce n’est pas l’intention du code que vous avez paraphrasé.

L’écriture trop de code anti-pattern. Cela rend plus difficile la lecture et la compréhension.

Comme il a déjà été souligné, il serait préférable d’utiliser une interface.

Fondamentalement, les programmeurs ne tirent pas parti des principes orientés object et font plutôt des choses en utilisant un code procédural. Chaque fois que nous recherchons la déclaration “if”, nous devrions nous demander si nous ne devrions pas utiliser un concept OO au lieu d’écrire davantage de code procédural.

C’est juste un mauvais code, il n’a pas de nom (il n’a même pas de design OO). Mais l’argument pourrait être que le premier code ne jette pas en avant le principe d’ouverture . Que se passe-t-il lorsque la liste des choses sockets en charge change? Vous devez réécrire la méthode que vous utilisez.

Mais la même chose se produit lorsque vous utilisez le second extrait de code. Disons que la règle de support change, vous devrez aller à chacune des méthodes et les réécrire. Je vous suggère d’avoir une classe de support abstraite et de passer différentes règles de support quand elles changent.

Je ne pense pas qu’il ait un nom, mais peut-être vérifier la liste maîtresse à http://en.wikipedia.org/wiki/Anti-pattern sait? http://en.wikipedia.org/wiki/Hard_code semble probablement plus proche.

Je pense que votre exemple n’a probablement pas de nom – alors que la solution que vous proposez s’appelle Composite .

http://www.dofactory.com/Patterns/PatternComposite.aspx

Comme vous ne montrez pas vraiment ce qu’est le code, il est difficile de vous donner une bonne dose de sensations. En voici un qui n’utilise aucune clause if .

 // invoked to map different kinds of items to different features public void BootStrap { featureService.Register(typeof(MyItem), new CustomFeature()); } // your code without any ifs. public void ControlStuff() { var listOfThings = LoadThings(); foreach (var thing in listOfThings) { thing.InvokeFeatures(); } } // your object interface IItem { public ICollection Features {get;set;} public void InvokeFeatues() { foreach (var feature in Features) feature.Invoke(this); } } // a feature that can be invoked on an item interface IFeature { void Invoke(IItem container); } // the "glue" public class FeatureService { void Register(Type itemType, IFeature feature) { _features.Add(itemType, feature); } void ApplyFeatures(T item) where T : IItem { item.Features = _features.FindFor(typof(T)); } } 

Je l’appellerais un Failure to Encapsulate . C’est un terme inventé, mais c’est réel et vu assez souvent

Beaucoup de gens oublient que l’encapsulation ne se limite pas au masquage des données avec un object, mais également au fait de cacher un comportement au sein de cet object, ou plus précisément au masquage de la manière dont le comportement d’un object est implémenté.

En ayant un object externe DoSomething() , nécessaire au bon fonctionnement du programme, vous créez beaucoup de problèmes. Vous ne pouvez pas raisonnablement utiliser l’inheritance dans votre liste de choses. Si vous modifiez la signature de la “chose”, dans ce cas la chaîne, le comportement ne suit pas. Vous devez modifier cette classe externe pour append son comportement (appel de DoSomething() à la thing dérivée).

Je proposerais la solution “améliorée”, qui consiste à avoir une liste d’objects Thing , avec une méthode qui implémente DoSomething() , qui agit comme une NOOP pour les choses qui ne font rien. Cela localise le comportement de la thing en elle-même, et la maintenance d’une liste de correspondance spéciale devient inutile.

Si c’était une chaîne, je pourrais l’appeler “chaîne magique”. Dans ce cas, je considérerais “tableau de chaînes magiques”.

Je ne sais pas s’il existe un «modèle» pour écrire du code qui n’est pas maintenable ou réutilisable. Pourquoi ne peux-tu pas leur en donner la raison?

Pour moi, le mieux est d’expliquer cela en termes de complexité de calcul. Dessinez deux graphiques montrant le nombre d’opérations requirejses en terme de count(listOfThingsThatSupportX ) et count(listOfThings ) et comparez avec la solution que vous proposez.

Au lieu d’utiliser des interfaces, vous pouvez utiliser des atsortingbuts. Ils décriraient probablement que l’object devrait être “taggé” comme ce type d’object, même si le tagging en tant que tel n’introduit aucune fonctionnalité supplémentaire. Un object décrit comme “Chose A” ne signifie pas que toutes les “Choses A” ont une interface spécifique, il est juste important qu’elles soient une “Chose A”. Cela semble être le travail des atsortingbuts plus que des interfaces.