这个坏实践/反模式的名称是什么?

本文关键字：是什么 模式 | 更新日期: 2023-09-27 18:10:28

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

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

所以，很明显为什么第一种方法是不好的做法，但是这叫什么呢?还有，有没有比我建议的更适合这个问题的模式?

这个坏实践/反模式的名称是什么?

通常一个更好的方法(IMHO)是使用接口而不是继承

则只需检查对象是否实现了接口。

我认为反模式名称是硬编码:)

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

更常见的是，X可能是ThingBase可能需要使用ThingBase.supports(ThingBaseProperty)或其他类似的东西来暴露其功能的情况之一。

在我看来，这里起作用的基本设计原则是封装。在您建议的解决方案中，您已经将逻辑封装在Thing类中，而在原始代码中，逻辑泄漏到调用者中。

它也违反了开闭原则，因为如果您想添加支持X的新子类，那么现在需要去并修改包含硬编码列表的任何地方。在您的解决方案中，您只需添加新类，重写方法，就完成了。

不知道名称(怀疑存在)，但将每个"事物"想象为一辆汽车-有些汽车有巡航控制系统，而其他汽车没有。

现在你有一个车队，你想知道哪些车有巡航控制。

使用第一种方法就像找到所有有巡航控制的车型的列表，然后一辆一辆地搜索列表中的每一辆车——如果有，就意味着这辆车有巡航控制，否则就没有。麻烦,对吧?

使用第二种方法意味着每辆有巡航控制系统的汽车都有一个标签，上面写着"我有巡航控制系统"，你只需要寻找这个标签，而不依赖外部来源给你信息。

不是很专业的解释，但简单扼要。

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

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

正如已经指出的，使用接口会更好。

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

这只是一个糟糕的代码，它没有一个名字(它甚至没有一个OO设计)。但论点可能是，第一个代码不遵循开合原则。当支持的东西列表发生变化时会发生什么?你必须重写你正在使用的方法。

但是当您使用第二个代码片段时也会发生同样的事情。假设支持规则发生了变化，那么您就必须找到每个方法并重新编写它们。我建议你有一个抽象的支持类，并通过不同的支持规则时，他们的变化。

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

我想你的例子可能没有名字——而你提出的解决方案有，它被称为Composite。

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

由于您没有显示代码的真正用途，因此很难给您一个健壮的解决方案。这里有一个根本没有使用任何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));
    }
}

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

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

如果有一个外部DoSomething()，这是正确的程序操作所必需的，那么就会产生很多问题。你不能在你的清单中合理地使用继承。如果你改变了"东西"的签名，在这个例子中是字符串，行为就不遵循了。您需要修改这个外部类以添加它的行为(将DoSomething()调用回派生的thing)。

我将提供"改进"的解决方案，即有一个Thing对象列表，并使用实现DoSomething()的方法，该方法充当不做任何事情的NOOP。这将使thing的行为本地化，并且不需要维护特殊的匹配列表。

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

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

对我来说，最好的解释是在计算复杂性方面。用count(listOfThingsThatSupportX )和count(listOfThings )绘制两张图表，显示所需的操作次数，并与您提出的解决方案进行比较。

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

相关文章：