What is the name of this bad practice / anti-patte

Question

I'm trying to explain to my team why this is bad practice, and am looking for an anti-pattern reference to help in my explanation. This is a very large enterprise app, so here's a simple example to illustrate what was implemented:

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

I'm suggesting that we add a property to the 'Things' base class to tell us if it supports X, since the Thing subclass will need to implement the functionality in question. Something like this:

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
{
}

So, it's pretty obvious why the first approach is bad practice, but what's this called? Also, is there a pattern better suited to this problem than the one I'm suggesting?

Answer 1

I think the anti-pattern name is hard-coding :)

Whether there should be a ThingBase.supportsX depends at least somewhat on what X is. In rare cases that knowledge might be in ControlStuff() only.

More usually though, X might be one of set of things in which case ThingBase might need to expose its capabilities using ThingBase.supports(ThingBaseProperty) or some such.

Answer 2

Normally a better approach (IMHO) would be to use interfaces instead of inheritance

then it is just a matter of checking whether the object has implemented the interface or not.

Answer 3

Instead of using interfaces, you could use attributes. They would probably describe that the object should be 'tagged' as this sort of object, even if tagging it as such doesn't introduce any additional functionality. I.e. an object being described as 'Thing A' doesn't mean that all 'Thing A's have a specific interface, it's just important that they are a 'Thing A'. That seems like the job of attributes more than interfaces.

Answer 4

There is a perfectly reasonable situation where this coding practice makes sense. It might not be an issue of which things actually support X (where of course an interface on each thing would be better), but rather which things that support X are ones that you want to enable. The label for what you see is then simply configuration, presently hard-coded, and the improvement on this is to move it eventually to a configuration file or otherwise. Before you persuade your team to change it I would check this is not the intention of the code you have paraphrased.

Answer 5

Don't know about a name (doubt such exists) but think of each "Thing" as a car - some cars have Cruise Control system and others do not have.

Now you have fleet of cars you manage and want to know which have cruise control.

Using the first approach is like finding list of all car models which have cruise control, then go car by car and search for each in that list - if there it means the car has cruise control, otherwise it doesn't have. Cumbersome, right?

Using the second approach means that each car that has cruise control come with a sticker saying "I has cruise control" and you just have to look for that sticker, without relying on external source to bring you information.

Not very technical explanation, but simple and to the point.

Answer 6

I don't know if there is a 'pattern' for writing code that is not maintainable or reusable. Why can't you just give them the reason?