这种方法是否违反SOLID或有代码味？

Question

这是否会产生任何代码异味或违反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原则？

Answer 1

我闻到了一些东西......

如果你的类都实现了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());

Answer 2

似乎IDisplayable应该有一个显示名称的方法,所以你可以将该方法减少到类似的东西

summary.Append("The " + item.displayName() + " is " + item.getInfo());

Answer 3

鉴于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。Add()此外，如果您重写and Remove()inMyCollection并让它们检查对象的类型并设置标志，您可能会获得更好的性能，因此您的containsHumans()函数（和其他函数）可以简单地返回标志的状态，而不必迭代集合每次他们被召唤时。