此方法是否违反SOLID或有代码味道？

这是否会给出任何代码异味或违反SOLID原则？此方法是否违反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(); 
}

正如你看到的，我的汇总（）绑定到实现类，如人类，动物等

是否闻到？我是否违反LSP？任何其他固体原则？

来源

2010-09-03 SP.

鉴于从OP上this answer的评论，我认为最好的方法是创建一个自定义容器类，以取代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(); 
}

当然我的例子是过于简单和做作。例如，无论是作为if/else语句中的逻辑的一部分，还是围绕整个块，您都需要遍历displayableItems集合。此外，如果您覆盖Add()和Remove(),MyCollection，并让它们检查对象的类型并设置标志，那么您的containsHumans()函数（和其他函数）可以简单地返回标志的状态，而不是每次调用集合时迭代集合。

来源

2010-09-03 16:10:21 rmeador

感谢您的回复，但我不完全了解技术性，所以您能否给我举个例子呢？ :)再次感谢，这可能正是我需要的。我只需要看看它的一个例子。 – 2010-09-03 18:28:11

嗯......你接受这个事实是否意味着你不需要一个例子？我很乐意尝试提供，但我不确定哪个部分会导致您感到困惑。如果你可以更具体，我会试试看。 – rmeador 2010-09-03 18:53:52

我接受它作为答案，因为它回答了我的具体问题。我正在研究基于此的解决方案。我只需要一个例子来确保我理解技术性。贾斯汀的回答非常详尽和有帮助，所以类似的东西会非常棒。 – 2010-09-03 18:57:54

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

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

来源

2010-09-03 15:48:32

谢谢，但是如果我必须在Summarize（）方法中使用某种逻辑来查看列表中的可显示项目是什么类型呢？我想我的问题是将Summarize（）与具体的实现类绑定是否是一个不好的做法。 – 2010-09-03 15:53:46

@SP - 是的。如果您将Summarize（）的行为绑定到具体的实现，那么您确实否定了通过IDisplayable接口多态处理对象的好处。如果Summarize中的行为需要更改，它应该通过IDisplayable接口的成员来完成。 – 2010-09-03 15:56:21

@SP，或许可以解释更多你想要做的事情。例如，IDisplayable可能有1000个实现类型，但您只关心其中的5个。 – 2010-09-03 15:58:03

好像IDisplayable应该有一个显示名称的方法，所以你可以在方法减少类似

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

来源

2010-09-03 15:50:01 Joey

欢迎来到StackOverflow，享受您的逗留，并记得提醒你的女服务员。也尽可能使用正确的代码格式！ – 2010-09-03 15:52:14

如何：

summary.Append("The " + item.getType() + " is " + item.GetInfo());

来源

2010-09-03 15:50:54 froadie

是的。

为什么不让每一个类从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()); 
}

来源

2010-09-03 15:51:39

在它违反了LSP和开路最低封闭原则。

解决的办法是有一个Description属性的IDisplayable接口，这样总结可以叫

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

这也可以通过反射解决的，因为你只得到了类的名称。

更好的解决办法是从GetInfo()方法返回类似IDisplayableInfo的东西。这将有助于保持OCP的可扩展性。

来源

2010-09-03 15:53:29 Jay

您能否简要解释为什么它违反了LSP？我知道这违反了OCP。如果我在Summarize（）中有额外的逻辑来检查那里有什么类型的实现类呢？ – 2010-09-03 15:59:32

@SP这里可能会有细微的区别。这个原则的一个常见方式是“使用指针或对基类的引用的函数必须能够在不知道它的情况下使用派生类的对象。”“不知道它”是什么意思？从某种意义上说，你的方法可以处理任何“IDisplayable”的实现 - 它会编译并且不会抛出异常。在另一个意义上说，它不起作用，因为它必须“知道”实现，才能对“IDisplayable”做一些有意义的事情。 – Jay 2010-09-03 16:24:59

如果您无法修改IDisplayable或类实现，并且您使用的是.NET 3.5或更高版本，则可以使用扩展方法。但是，这并不是说要好得多

来源

2010-09-03 15:54:08

此方法是否违反SOLID或有代码味道？

回答

相关问题