这个坏习惯/反模式的名称是什么？

Question

我正在尝试向我的团队解释为什么这是不良实践，并寻找反模式参考来帮助我进行解释。这是一个非常大的企业应用程序，因此这里有一个简单的示例来说明所实现的内容：

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

我建议我们在“Things”基类中添加一个属性，以告诉我们它是否支持X，因为Thing子类需要实现相关的功能。像这样：

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

所以，第一种方法为什么不好是很明显的，但这被称为什么？此外，有没有比我建议的更适合这个问题的模式？

- John Cornell

---

根据X的不同，你的建议可能会因为不同的原因与硬编码O(n^2)方法一样具有问题。你将关于X的知识强制放入基类中，因此在创建每个派生类时（无论它是否支持X）都需要这些知识。对于那些与X完全无关的“Thing”类型，"SupportsX()"方法只是公共接口中的污染。如果你有许多不同的特性(SupportsY(), SupportsZ()等)，这种污染会很快变得极其严重。可以考虑使用泛型测试Supports(X)或者使用接口来标记特性支持。 - Lee

Answer 1

通常更好的方法（个人意见）是使用接口而不是继承。

然后只需要检查对象是否实现了接口即可。

- AndersK

---

同意：OP建议的方式意味着您需要事先了解X，并编写SupportsX。使用这种方式，您只需在派生类中实现ISomething即可，并且可以使用if (myObj is ISomething)来查看它是否支持它。 - Kieren Johnstone

我完全支持你的观点，OP提出的两种方法都不是很好地解决问题。应该有一个ISupportX接口，如果相应的“事物”支持它，则实现该接口。否则ThingBase和所有派生的Thing类就会变得臃肿。这不符合面向对象的“关注点分离”原则。 - Andreas

"myObj is ISomething" 可能违反了 Liskov 替换原则。 - jgauffin

检查对象是否实现了接口，尽管这种方法很普遍，但并不是一个好的做法。 - Miserable Variable

@HemalPandya为什么这是件坏事？ - wprl

Answer 2

我认为这种反模式叫做硬编码 :)

是否应该有一个ThingBase.supportsX取决于X是什么。在罕见的情况下，这种知识可能仅存在于ControlStuff（）中。

然而更通常的情况是，X可能是一组事物之一，在这种情况下，ThingBase可能需要使用ThingBase.supports（ThingBaseProperty）或类似方法来公开其功能。

- Miserable Variable

Answer 3

在这里起作用的基本设计原则是封装。在您提出的解决方案中，您已将逻辑封装在Thing类内部，而在原始代码中，逻辑泄漏到调用方。

这也违反了开闭原则，因为如果您想添加支持X的新子类，则现在需要修改包含该硬编码列表的任何位置。使用您的解决方案，只需添加新类，重写方法即可完成。

- Dylan Smith

Answer 4

不知道是否有一个名字（怀疑这种存在），但可以将每个“Thing”视为一辆汽车 - 有些汽车有巡航系统，而其他汽车则没有。

现在你有一批汽车需要管理，并想知道哪些有巡航控制。

使用第一种方法就像找到所有拥有巡航控制的汽车型号列表，然后逐个检查每辆汽车，搜索每个型号是否在列表中 - 如果在列表中，则表示该车具有巡航控制，否则它没有。很麻烦，对吧？

使用第二种方法意味着每个有巡航控制的汽车都带有一个标签，上面写着“我有巡航控制”，你只需寻找那个标签，而不依赖外部信息来获取信息。

虽然不是非常技术性的解释，但是简单明了。

- Shadow Wizard Chasing Stars

---

第二种方法的缺点是，所有汽车都会贴有标签，标明它们是否具备巡航控制功能，并且这个标签是永久性的。 - thedaian

@thedaian 为什么这是一个缺点？贴纸可以轻松地被移除（将其设置为“false”）。没有贴纸，就没有巡航控制了。 - Shadow The Spring Wizard

这个贴纸仍然在那里，把它设置为false只会将文本从“我有巡航控制”更改为“我没有巡航控制”，SupportsX函数仍然存在，不受任何影响。 - thedaian

@thedaian 好的，让我们转向编程。在你看来，为什么始终拥有属性/函数会有缺点？ - Shadow The Spring Wizard

其他答案已经解释了为什么这是一个缺点，但基本上：您正在向类中添加一堆额外的函数，其中一些可能只使用一两次，用于确定对象是否支持X的代码仍然是硬编码的并且分散在多个文件中。在这里，一组接口将是更好的选择。 - thedaian

好的，说得对。谢谢你的见解！ :-) - Shadow The Spring Wizard

Answer 5

这种编码实践在某些情况下是完全合理的。它可能并不涉及到哪些事物实际上支持X（当然，每个事物都有一个接口会更好），而是涉及到哪些支持X的事物是您想要启用的。您看到的标签只是配置，目前是硬编码的，改进方法是将其最终移动到配置文件或其他位置。在说服团队进行更改之前，请确保这不是您已经转述的代码的意图。

- Benedict

Answer 6

写太多代码反模式。它使得阅读和理解变得更加困难。

正如已经指出的那样，最好使用接口。

基本上，程序员没有充分利用面向对象原则，而是使用过程化代码来完成任务。每次我们使用“if”语句时，都应该问自己是否应该使用面向对象的概念，而不是写更多的过程化代码。

- Sarel Botha

---

那种反模式确实有一个名字，它叫做“功能分解”，与“对象分解”相对比。 - amoss

Answer 7

这只是一段糟糕的代码，没有名称（甚至没有面向对象的设计）。但可以认为第一段代码没有遵循 开闭原则。如果支持的列表更改会发生什么？您必须重写正在使用的方法。

但是当您使用第二个代码片段时，同样的事情会发生。假设支持规则发生变化，您需要到每个方法中进行重写。我建议您拥有一个抽象的Support类，并在更改时传递不同的支持规则。

- Mykolas Simutis

---

确实，这种模式/反模式在很大程度上取决于上下文。 - Kromster

Answer 8

既然你没有展示真正的代码，那么很难给出一个健壮的解决方案。这里提供了一种不使用任何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<IFeature> 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>(T item) where T : IItem
    {
        item.Features = _features.FindFor(typof(T));
    }
}

- jgauffin

---

或许我是个新手，或者这有点过于复杂了。可能两者都有一点吧。 - Phil

为什么它被过度设计了？任何需要使用if/switch语句的解决方案都需要在所有需要支持新功能的地方进行手动代码更改。而这个解决方案则没有这个问题。 - jgauffin

你说得对，这样做将来需要更少的代码更改。然而，所有这一切的结构似乎有点复杂，不太容易理解它所试图实现的目标。对于某些情况，这可能非常合适，但对于大多数情况，我会倾向于YAGNI。 - Phil

作为一则侧记，你的话比我的更有分量 - 我查看了你的个人资料，发现你比我经验丰富得多。这只是我的第一印象。 - Phil

这只是我的个人意见：我认为在不需要显式要求易变性的情况下，易于理解优先于易于更改。在我看来，这个解决方案非常复杂。因此，我同意Phil的看法，认为它过度工程化了。 - Dunk

Answer 9

我不认为它有一个名称，但是也许可以在http://en.wikipedia.org/wiki/Anti-pattern的主列表中查找。http://en.wikipedia.org/wiki/Hard_code可能更接近。

我认为你的例子可能没有名称 - 而你提出的解决方案被称为组合模式。 http://www.dofactory.com/Patterns/PatternComposite.aspx

- Dave Walker

---

硬编码肯定是其中的一部分，可以轻松地移动到配置文件中，但这仍然是需要维护的额外内容。那么SoC呢？ - John Cornell

Answer 10

我将称之为“未封装失败”。虽然这是一个虚构的术语，但它确实存在并经常出现。

很多人忘记了封装不仅仅是隐藏对象中的数据，还包括隐藏对象中的行为，或更具体地说，是隐藏对象行为的实现方式。

通过使用外部的DoSomething()函数来保证程序的正确运行，会产生很多问题。你无法在物品列表中合理地使用继承。如果你改变“thing”(在本例中是字符串)的签名，行为不会随之改变。你需要修改该外部类以添加其行为（将DoSomething()回调到派生的thing）。

我建议采用“改进”的解决方案，即使用一系列Thing对象的列表，并实现一个执行DoSomething()函数的方法，对于不执行任何动作的物品，可以将其作为NOOP处理。这样就把thing的行为局限在其内部，不再需要维护特殊的匹配列表。

- Edwin Buck