logo标签列表

将方法绑定到实现类

c#oopdesign-patterns
11

这是否有任何代码气味或违反了SOLID原则？

public string Summarize()
{
IList<IDisplayable> displayableItems = getAllDisplayableItems();
StringBuilder summary = new StringBuilder();

foreach(IDisplayable item in displayableItems)
{
    if(item is Human)
        summary.Append("The person is " + item.GetInfo());

    else if(item is Animal) 
        summary.Append("The animal is " + item.GetInfo());

    else if(item is Building) 
        summary.Append("The building is " + item.GetInfo());

    else if(item is Machine) 
        summary.Append("The machine is " + item.GetInfo());
}

return summary.ToString();
}

从代码中可以看出，我的Summarize()方法与实现类（如Human、Animal等）紧密耦合。

这段代码是否违反了LSP原则？（还有其他SOLID原则吗？）

7个回答
21

我嗅到了一点什么......

如果你的类都实现了IDisplayable接口，它们应该各自实现自己的显示逻辑。这样你的循环就会变得更清晰简洁：

public interface IDisplayable
{
    void Display();
    string GetInfo();
}

public class Human : IDisplayable
{
    public void Display() { return String.Format("The person is {0}", 
        GetInfo());

    // Rest of Implementation
}

public class Animal : IDisplayable
{
    public void Display() { return String.Format("The animal is {0}", 
        GetInfo());

    // Rest of Implementation
}

public class Building : IDisplayable
{
    public void Display() { return String.Format("The building is {0}", 
        GetInfo());

    // Rest of Implementation
}

public class Machine : IDisplayable
{
    public void Display() { return String.Format("The machine is {0}", 
        GetInfo());

    // Rest of Implementation
}

那么你可以将你的循环改成更简洁的形式（并允许类实现其自己的显示逻辑）：

foreach(IDisplayable item in displayableItems)
    summary.Append(item.Display());
谢谢，但是如果我必须在Summarize()方法中有某种逻辑来查看列表中有哪些IDisplayable项，该怎么办？我的问题是，将Summarize()与具体实现类绑定是否是一种不好的做法。 - SP.
2
@SP - 是的。如果你把Summarize()方法的行为与具体实现绑定在一起，那么你就会显著削弱通过IDisplayable接口来进行多态处理的优势。如果Summarize方法的行为需要改变，应该通过IDisplayable接口的成员来实现。 - Justin Niessner
@SP，或许你可以更详细地解释一下你想要做什么。例如，IDisplayable 接口可能有 1000 种实现类型，但你只关心其中的 5 种。 - Anthony Pegram
1
也许我的例子过于简单，无法揭示我的意图。我“真正的”Summarize()不仅仅是显示，它还有逻辑来查看有哪些类型，例如人类、动物等等。我的Summarize()对于只有人类的情况有特定的逻辑，但如果同时有人类和动物，则执行其他操作。因此，你可以看到它含有逻辑。 - SP.
@SP - 如果是这样的话，您应该更新您的问题，以便我们更好地了解您正在做什么。 - Justin Niessner
显示剩余2条评论
5
似乎IDisplayable应该有一个显示名称的方法，这样你就可以将该方法简化为类似以下内容：
summary.Append("The " + item.displayName() + " is " + item.getInfo());
欢迎来到StackOverflow，愿您在这里过得愉快，记得给女服务员小费。同时，请尽可能使用正确的代码格式！ - Anthony Pegram
3

可以。

为什么不让每个类实现 IDisplayable 接口中的一个方法来展示它们的类型：

interface IDisplayable
{
    void GetInfo();
    public string Info;
}
class Human : IDisplayable
{
   public string Info
   { 
    get 
    { 
        return "";//your info here
    }
    set;
   }

   public void GetInfo()
   {
       Console.WriteLine("The person is " + Info)
   }
}

然后只需按照以下方式调用您的方法：

foreach(IDisplayable item in displayableItems)
{
    Console.WriteLine(item.GetInfo());
}
1

至少违反了LSP和开闭原则。

解决方案是在IDisplayable接口中添加一个Description属性，这样summarize只需调用该属性即可。

summary.Append(string.Format("The {0} is {1}", item.Description, item.GetInfo()));

这个问题也可以通过反射来解决，因为你只需要获取类的名称。

更好的解决方案是从GetInfo()方法返回像IDisplayableInfo这样的东西。这将是一个可扩展性点，有助于保持OCP。

你能简要解释一下为什么它违反了LSP吗？我知道它违反了OCP。如果我在Summarize()中有额外的逻辑来检查有哪些实现类，会怎样呢？ - SP.
@SP 这里可能需要做出微妙的区分。原则通常是这样陈述的：使用指向基类的指针或引用的函数必须能够在不知道它的情况下使用派生类的对象。“不知道它”是什么意思？从某种意义上说，您的方法确实可以使用任何 IDisplayable 的实现——它将编译并且不会抛出异常。但从另一个角度来看，它不起作用，因为它必须“知道”实现才能对 IDisplayable 做出有意义的操作。 - Jay
1

鉴于OP在这个答案中的评论，我认为最好的方法是创建一个自定义容器类来替换IList<IDisplayable> displayableItems，该类具有像containsHumans()和containsAnimals()这样的方法，因此您可以将不良的非多态代码封装在一个地方，并保持Summarize()函数的逻辑清晰。

class MyCollection : List<IDisplayable>
{
    public bool containsHumans()
    {
        foreach (IDisplayable item in this)
        {
            if (item is Human)
                return true;
        }

        return false;
    }

    // likewise for containsAnimals(), etc
}

public string Summarize()
{
    MyCollection displayableItems = getAllDisplayableItems();
    StringBuilder summary = new StringBuilder();

    if (displayableItems.containsHumans() && !displayableItems.containsAnimals())
    {
        // do human-only logic here
    }
    else if (!displayableItems.containsHumans() && displayableItems.containsAnimals())
    {
        // do animal-only logic here
    }
    else
    {
        // do logic for both here
    }

    return summary.ToString();
}

当然，我的例子过于简单和人为。例如，在您的Summarize() if / else语句的逻辑中，或者可能围绕整个块，您将需要迭代displayableItems集合。此外，如果在MyCollection中覆盖Add()和Remove()并检查对象的类型并设置标志，那么您的containsHumans()函数（和其他函数）可以简单地返回标志的状态而无需每次调用时迭代集合，这样可以获得更好的性能。

谢谢你的回复，但是我还没有完全理解其中的技术性，请你给我一个例子好吗？ :) 再次感谢，这也许是我需要的东西。我只需要看一个例子。 - SP.
嗯...你接受了这个意味着你不需要一个例子吗？我很乐意尝试提供一个，但我不太确定哪一部分让你困惑了。如果你能更具体地说明，我会尝试解答。 - rmeador
我接受了这个答案，因为它回答了我的具体问题。我正在基于此解决问题。我只需要一个示例来确保我理解技术细节。Justin下面的回答非常详细且有帮助，所以类似的东西会很棒。 - SP.
@SP：我添加了一个例子...如果需要进一步帮助，请告诉我。 - rmeador
非常感谢！这确实使事情变得更加清晰。我有一个问题，就是Summarize()现在与MyCollection类绑定在一起了。我想我可能对此过于担心了，但是我是否可以有像IMyCollection接口这样的东西，以便Summarize()不会与具体的MyCollection实现绑定？再次感谢您的帮助！ - SP.
@SP：当然可以像您描述的那样引入一个接口。 - rmeador
1
怎么样：
    summary.Append("The " + item.getType() + " is " + item.GetInfo());
0

如果您无法修改IDisplayable或类实现，并且正在使用.NET 3.5或更高版本，则可以使用扩展方法。但这并没有太大的改善。

网页内容由stack overflow 提供, 点击上面的
可以查看英文原文，
原文链接