这种不良做法/反模式的名称是什么？答案

【问题标题】：What is the name of this bad practice / anti-pattern?这种不良做法/反模式的名称是什么？
【发布时间】：2011-12-02 00:52:05
【问题描述】：

我正在尝试向我的团队解释为什么这是不好的做法，并且正在寻找反模式参考来帮助我解释。这是一个非常大的企业应用程序，所以这里有一个简单的例子来说明实现了什么：

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

那么，很明显为什么第一种方法是不好的做法，但是这叫什么？另外，有没有比我建议的更适合这个问题的模式？

【问题讨论】：

根据 X 是什么，您的建议可能与硬编码 O(n^2) 方法一样有问题，但原因不同。您将 X 的知识强加到基类中，因此也强加到您创建的每个派生类中（无论它是否支持 X）。对于 X 完全不相关的Things 类型，SupportsX() 方法只不过是公共接口中的污染。如果你有很多不同的功能（SupportsY()、SupportsZ() 等），这种污染很快就会变得极端。考虑一个通用测试Supports(X)，或者使用接口来标记功能支持。

标签： c# anti-patterns

【解决方案1】：

通常更好的方法（恕我直言）是使用接口而不是继承

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

【讨论】：

同意：OP建议的方式意味着您需要提前了解X，并编写SupportsX。这样你只需在派生类中实现 ISomething 并可以通过if (myObj is ISomething) 来查看它是否支持它
我完全支持你，OP 的两种方法都不能很好地解决问题。应该有一个 ISupportX 接口，如果它们支持，相应的“事物”就会实现它。否则 ThingBase 和所有派生的 Things 都会变成胖类。这不符合面向对象的“关注点分离”原则。
myObj is ISomething 可能违反了里氏替换原则。
检查对象是否实现了接口，虽然猖獗，却是一件坏事。
@HemalPandya 为什么这是一件坏事？

【解决方案2】：

我认为反模式名称是hard-coding :)

是否应该存在ThingBase.supportsX 至少在某种程度上取决于X 是什么。 在极少数情况下，知识可能只存在于ControlStuff() 中。

但更常见的是，X 可能是其中一种情况，在这种情况下，ThingBase 可能需要使用 ThingBase.supports(ThingBaseProperty) 或类似的方式公开其功能。

【讨论】：

【解决方案3】：

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

它也违反了开闭原则，因为如果你想添加支持 X 的新子类，你现在需要去修改包含该硬编码列表的任何地方。使用您的解决方案，您只需添加新类，覆盖该方法即可。

【讨论】：

【解决方案4】：

不知道名称（怀疑是否存在），但将每个“事物”视为汽车 - 有些汽车有巡航控制系统，有些则没有。

现在您拥有自己管理的车队，并想知道哪些车辆具有巡航控制功能。

使用第一种方法就像查找所有具有巡航控制功能的车型的列表，然后逐个驾驶汽车并搜索该列表中的每一个 - 如果存在则意味着汽车具有巡航控制，否则它没有。很麻烦吧？

使用第二种方法意味着每辆配备巡航控制系统的汽车都带有一个标有“我有巡航控制系统”的标签，您只需查找该标签即可，无需依赖外部资源为您提供信息。

不是很技术性的解释，但简单明了。

【讨论】：

第二种方法的缺点是所有的汽车都贴有标明是否有巡航控制的贴纸，并且该贴纸是永久性的。
@thedaian 为什么这是一个缺点？贴纸可以轻松移除（设置为false）。没有贴纸，没有巡航控制系统..
贴纸还在，设置为false只是将文字从“我有巡航控制”更改为“我没有巡航控制”，不管怎样，SupportsX的功能仍然存在。
@thedaian 好的，让我们再谈一谈编程。为什么在你看来总是有属性/功能有缺点？
其他答案已经涵盖了为什么它是一个缺点，但基本上：你正在向类中添加一堆额外的函数，其中一些可能只使用一次或两次，代码是否对象支持 X 仍然是硬编码的并且分布在多个文件中。一组接口在这里会是更好的选择。

【解决方案5】：

在完全合理的情况下，这种编码实践是有意义的。这可能不是哪些东西真正支持 X 的问题（当然，每个东西都有一个接口会更好），而是哪些支持 X 的东西是您想要启用的东西。你所看到的标签只是简单的配置，目前是硬编码，对此的改进是将其最终移动到配置文件或其他文件中。在你说服你的团队改变它之前，我会检查这不是你所解释的代码的意图。

【讨论】：

【解决方案6】：

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

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

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

【讨论】：

那个反模式确实有一个名字，叫做“功能分解”，与“对象分解”形成对比。

【解决方案7】：

这只是一个糟糕的代码，它没有名字（它甚至没有面向对象的设计）。但争论可能是第一个代码没有闲置Open Close Principle。当支持的事物列表发生变化时会发生什么？你必须重写你正在使用的方法。

但是当您使用第二个代码 sn-p 时会发生同样的事情。假设支持规则发生变化，您必须转到每个方法并重写它们。我建议你有一个抽象的支持类，并在它们发生变化时传递不同的支持规则。

【讨论】：

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

【解决方案8】：

我不认为它有名字，但也许检查http://en.wikipedia.org/wiki/Anti-pattern 的主列表知道吗？ http://en.wikipedia.org/wiki/Hard_code 可能看起来更近了。

我认为您的示例可能没有名称 - 而您提出的解决方案则称为 Composite。

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

【讨论】：

硬代码肯定是其中的一部分，它很容易移动到配置文件中，但这仍然是需要维护的额外内容。 SoC 怎么样？

【解决方案9】：

由于您没有显示代码的真正含义，因此很难为您提供强大的支持。这是一个根本不使用任何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));
    }
}

【讨论】：

要么我是菜鸟，要么这有点过度设计。可能两者都有一点。
为什么会过度设计？任何需要if/switch 语句的解决方案都需要在所有必须支持新功能的地方手动更改代码。这个解决方案没有这个问题。
你是对的，这将需要更少的代码更改。然而，这一切的结构对于它试图实现的目标来说似乎有点复杂，而且更难理解。对于某些情况，这非常合适，但对于大多数情况，我会倾向于 YAGNI。
作为我的旁注，你的话胜过我的 - 查看你的个人资料，你的经验比我多得多。这只是我的第一印象。
只是我的 2 美分：我认为在任何不明确要求易于更改的情况下，易于理解胜过易于更改。 IMO，这个解决方案非常复杂。所以我同意菲尔的观点，因为它被过度设计了。

【解决方案10】：

我称之为Failure to Encapsulate。这是一个虚构的术语，但它是真实的并且经常看到

很多人忘记了封装不仅仅是隐藏对象中的数据，它也是隐藏对象中的行为，或者更具体地说，隐藏对象的行为是如何实现的。

通过拥有正确的程序操作所需的外部DoSomething()，您会产生很多问题。你不能在你的事物列表中合理地使用继承。如果您更改“事物”的签名，在这种情况下为字符串，则行为不会遵循。你需要修改这个外部类来添加它的行为（调用DoSomething()回到派生的thing。

我会提供“改进”的解决方案，即拥有一个Thing 对象列表，以及一个实现DoSomething() 的方法，该方法对于什么都不做的事情充当NOOP。这将thing 的行为本地化在自身内部，并且无需维护特殊的匹配列表。

【讨论】：

【解决方案11】：

如果它是一个字符串，我可能会称之为“魔术字符串”。在这种情况下，我会考虑“魔术字符串数组”。

【讨论】：

【解决方案12】：

我不知道是否存在编写不可维护或不可重用代码的“模式”。为什么不能直接告诉他们原因？

【讨论】：

我有，无论如何它都是这样实现的。也许这个例子会有所帮助，因为很难“只见树木不见森林”。谢谢。
为什么要投赞成票？我会说这是一个糟糕的答案，它对这个问题没有任何帮助。解释为什么它不可维护或不可重用会有所帮助。现在它只是重复说“这是一个糟糕的代码”的问题，任何具有至少一些 OO 技能的人都可以看到。我认为它应该得到 -1 而不是 +1。
操作员询问是否有反模式，我不相信上面的代码有反模式。 op 显然知道为什么代码是坏代码，那么我为什么要添加它呢？与其抱怨我的回答，不如提供一个或补充问题？
“我的解释中的帮助参考”。所以他实际上问为什么代码不好。如果没有名称，并不意味着无法解释。我确实给出了自己的想法，如何向团队解释。

【解决方案13】：

在我看来，最好的办法是用计算复杂度来解释这一点。用count(listOfThingsThatSupportX ) 和count(listOfThings ) 绘制两张图表，显示所需的操作数，并与您提出的解决方案进行比较。

【讨论】：

【解决方案14】：

您可以使用属性，而不是使用接口。他们可能会描述该对象应该被“标记”为这种对象，即使这样标记它不会引入任何额外的功能。 IE。被描述为“事物 A”的对象并不意味着所有“事物 A”都有特定的接口，重要的是它们是“事物 A”。这似乎是属性而不是接口的工作。

【讨论】：