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

Question

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

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

Je suggère d'ajouter une propriété à la "Choses" de la classe de base pour nous dire si elle prend en charge X, puisque la Chose la sous-classe aura besoin pour mettre en œuvre la fonctionnalité en question. Quelque chose comme ceci:

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 assez évident pourquoi la première approche est une mauvaise pratique, mais qu'est-ce appelé? Aussi, est-il un modèle mieux adapté à ce problème que celui que je suggère?

Answer 1

Normalement une meilleure approche (à mon humble avis) serait d'utiliser des interfaces au lieu de l'héritage

ensuite c'est juste une question de vérifier si l'objet a mis en œuvre l'interface ou pas.

Answer 2

Je pense que l'anti-modèle de nom est de coder en dur :)

S'il doit y avoir un ThingBase.supportsX dépend au moins un peu sur ce qu' X . Dans de rares cas, que le savoir peut-être, en ControlStuff() seulement.

Plus généralement, même si, X pourrait être un ensemble de choses auquel cas ThingBase pourriez avoir besoin pour mettre en avant ses capacités à l'aide de ThingBase.supports(ThingBaseProperty) ou quelque chose du genre.

Answer 3

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

Il viole également la Ouvert-Fermé principe, car si vous voulez ajouter de nouvelles sous-classes qui prennent en charge X vous devez maintenant aller et modifier n'importe où qui contient la liste codée en dur. Avec votre solution, il suffit d'ajouter la nouvelle classe, remplacer la méthode et vous avez terminé.

Answer 4

Ne sais pas à propos d'un nom (le doute il en existe un), mais pensez à chaque "Chose" comme une voiture - certaines voitures ont régulateur de vitesse et d'autres n'ont pas.

Maintenant, vous avez la flotte de voitures vous gérer le et que vous voulez savoir qui ont régulateur de vitesse.

En utilisant la première approche, c'est comme trouver la liste de tous les modèles de voiture qui ont régulateur de vitesse, puis aller de voiture en voiture et de recherche pour chaque dans cette liste - si cela signifie que la voiture a un régulateur de vitesse, sinon il n'a pas. Lourd, pas vrai?

À l'aide de la deuxième approche signifie que chaque voiture qui a le régulateur de vitesse de venir avec un autocollant disant "je a cruise control" et vous avez juste à regarder pour que l'autocollant, sans compter sur la source externe pour vous apporter l'information.

Pas très explication technique, mais simple et au point.

Answer 5

Il est parfaitement raisonnable de la situation où cette pratique de codage du sens. Il pourrait ne pas être un problème dont les choses support X (où bien sûr une interface sur chaque chose serait mieux), mais des choses qui prennent en charge les X sont ceux que vous souhaitez activer. L'étiquette de ce que vous voyez est alors simplement de configuration, actuellement codé en dur, et l'amélioration sur ce est de le déplacer éventuellement un fichier de configuration ou autre. Avant de vous persuader que votre équipe pour le changer, je voudrais vérifier ce n'est pas l'intention de le code que vous avez pour eux.