blocks|key|1733246|text|我闻到了一点东西..。|type|unstyled|depth|inlineStyleRanges|entityRanges|data|1733247|如果您的类都实现了IDisplayable，那么它们都应该实现自己的逻辑来显示自己。这样你的循环就会变得更干净：|1733248|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
}|code-block|syntax|javascript|1733249|然后，您可以将您的循环更改为更清晰的东西(并允许类实现自己的显示逻辑)：|1733250|foreach(IDisplayable+item+in+displayableItems)
++++summary.Append(item.Display());|1733251|entityMap^0|0|0|0|0|0^^$0|@$1|2|3|4|5|6|7|O|8|@]|9|@]|A|$]]|$1|B|3|C|5|6|7|P|8|@]|9|@]|A|$]]|$1|D|3|E|5|F|7|Q|8|@]|9|@]|A|$G|H]]|$1|I|3|J|5|6|7|R|8|@]|9|@]|A|$]]|$1|K|3|L|5|F|7|S|8|@]|9|@]|A|$G|H]]|$1|M|3|-4|5|6|7|T|8|@]|9|@]|A|$]]]|N|$]]

I smell a little something...

If your classes all implement IDisplayable, they should each implement their own logic to display themselves. That way your loop would change to something much cleaner:

<pre><code>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
}
</code></pre>

Then you can change your loop to something much cleaner (and allow the classes to implement their own display logic):

<pre><code>foreach(IDisplayable item in displayableItems)
 summary.Append(item.Display());
</code></pre>

blocks|key|984427|text|看起来IDisplayable应该有一个用于显示名称的方法，所以您可以将该方法简化为如下所示|type|unstyled|depth|inlineStyleRanges|entityRanges|data|984428|summary.Append("The+"+%2B+item.displayName()+%2B+"+is+"+%2B+item.getInfo());|code-block|syntax|javascript|984429|entityMap^0|0|0^^$0|@$1|2|3|4|5|6|7|I|8|@]|9|@]|A|$]]|$1|B|3|C|5|D|7|J|8|@]|9|@]|A|$E|F]]|$1|G|3|-4|5|6|7|K|8|@]|9|@]|A|$]]]|H|$]]

seems like IDisplayable should have a method for the display name so you can reduce that method to something like 

<pre><code>summary.Append("The " + item.displayName() + " is " + item.getInfo());
</code></pre>

blocks|key|1733265|text|这样如何：|type|unstyled|depth|inlineStyleRanges|entityRanges|data|1733266|++++summary.Append("The+"+%2B+item.getType()+%2B+"+is+"+%2B+item.GetInfo());+|code-block|syntax|javascript|1733267|entityMap^0|0|0^^$0|@$1|2|3|4|5|6|7|I|8|@]|9|@]|A|$]]|$1|B|3|C|5|D|7|J|8|@]|9|@]|A|$E|F]]|$1|G|3|-4|5|6|7|K|8|@]|9|@]|A|$]]]|H|$]]

How about:

<pre><code> summary.Append("The " + item.getType() + " is " + item.GetInfo()); 
</code></pre>

blocks|key|4344372|text|是。|type|unstyled|depth|inlineStyleRanges|entityRanges|data|4344373|为什么不让每个类实现一个来自IDisplayable的方法来显示它们的类型：|offset|length|style|CODE|4344374|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+"+%2B+Info)
+++}
}|code-block|syntax|javascript|4344375|然后按如下方式调用您的方法：|4344376|foreach(IDisplayable+item+in+displayableItems)
{
++++Console.WriteLine(item.GetInfo());
}|4344377|entityMap^0|0|E|C|0|0|0|0^^$0|@$1|2|3|4|5|6|7|S|8|@]|9|@]|A|$]]|$1|B|3|C|5|6|7|T|8|@$D|U|E|V|F|G]]|9|@]|A|$]]|$1|H|3|I|5|J|7|W|8|@]|9|@]|A|$K|L]]|$1|M|3|N|5|6|7|X|8|@]|9|@]|A|$]]|$1|O|3|P|5|J|7|Y|8|@]|9|@]|A|$K|L]]|$1|Q|3|-4|5|6|7|Z|8|@]|9|@]|A|$]]]|R|$]]

Yes.

Why not have each class implement a method from <code>IDisplayable</code> that shows their type:

<pre><code>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)
 }
}
</code></pre>

Then just call your method as follows:

<pre><code>foreach(IDisplayable item in displayableItems)
{
 Console.WriteLine(item.GetInfo());
}
</code></pre>

blocks|key|2934499|text|至少它违反了LSP和Open-Closed原则。|type|unstyled|depth|inlineStyleRanges|entityRanges|data|2934500|解决方案是为IDisplayable接口设置一个Description属性，这样summarize就可以调用|offset|length|style|CODE|2934501|summary.Append(string.Format("The+{0}+is+{1}",+item.Description,+item.GetInfo()));|code-block|syntax|javascript|2934502|这也可以通过反射来解决，因为你只得到了类名。|2934503|更好的解决方案是从GetInfo()方法返回类似于IDisplayableInfo的内容。这将是一个帮助保留OCP的扩展点。|2934504|entityMap^0|0|6|C|O|B|0|0|0|9|9|P|G|0^^$0|@$1|2|3|4|5|6|7|S|8|@]|9|@]|A|$]]|$1|B|3|C|5|6|7|T|8|@$D|U|E|V|F|G]|$D|W|E|X|F|G]]|9|@]|A|$]]|$1|H|3|I|5|J|7|Y|8|@]|9|@]|A|$K|L]]|$1|M|3|N|5|6|7|Z|8|@]|9|@]|A|$]]|$1|O|3|P|5|6|7|10|8|@$D|11|E|12|F|G]|$D|13|E|14|F|G]]|9|@]|A|$]]|$1|Q|3|-4|5|6|7|15|8|@]|9|@]|A|$]]]|R|$]]

At a minimum it violates LSP and Open-Closed Principle.

The solution is to have a <code>Description</code> property to the <code>IDisplayable</code> interface, so that summarize can just call 

<pre><code>summary.Append(string.Format("The {0} is {1}", item.Description, item.GetInfo()));
</code></pre>

This could also be solved via reflection, since you're only getting the name of the class.

The better solution is to return something like an <code>IDisplayableInfo</code> from the <code>GetInfo()</code> method. This would be an extensibility point to help preserve OCP.

blocks|key|984478|text|如果您不能修改IDisplayable或类实现，而您使用的是.NET+3.5或更高版本，则可以使用扩展方法。但这也好不到哪里去|type|unstyled|depth|inlineStyleRanges|entityRanges|data|984479|entityMap^0|0^^$0|@$1|2|3|4|5|6|7|D|8|@]|9|@]|A|$]]|$1|B|3|-4|5|6|7|E|8|@]|9|@]|A|$]]]|C|$]]

If you can't modify IDisplayable or the class implementations and you're using .NET 3.5 or later you can use Extension methods. But that's not that much better

blocks|key|1729689|text|考虑到OP对this+answer的评论，我认为最好的方法是创建一个自定义容器类来替换具有containsHumans()和containsAnimals()等方法的IList<IDisplayable>+displayableItems，这样您就可以将讨厌的非多态代码封装在一个地方，并保持Summarize()函数中的逻辑清晰。|type|unstyled|depth|inlineStyleRanges|offset|length|style|CODE|entityRanges|data|1729690|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();
}|code-block|syntax|javascript|1729691|当然，我的例子过于简单和做作。例如，无论是作为Summarize()+if/else语句中逻辑的一部分，还是围绕整个代码块，您都需要遍历displayableItems集合。此外，如果在MyCollection中覆盖Add()和Remove()，并让它们检查对象的类型并设置标志，那么您可能会获得更好的性能，这样您的containsHumans()函数(和其他函数)就可以简单地返回标志的状态，而不必在每次调用它们时迭代集合。|1729692|entityMap|0|LINK|mutability|MUTABLE|url|https://stackoverflow.com/questions/3637377/does-this-method-violate-solid-or-has-code-smell/3637404#3637404^0|19|G|1Q|H|2B|10|42|B|6|B|0|0|0|N|B|1W|G|2L|C|30|5|36|8|4E|G|0^^$0|@$1|2|3|4|5|6|7|U|8|@$9|V|A|W|B|C]|$9|X|A|Y|B|C]|$9|Z|A|10|B|C]|$9|11|A|12|B|C]]|D|@$9|13|A|14|1|15]]|E|$]]|$1|F|3|G|5|H|7|16|8|@]|D|@]|E|$I|J]]|$1|K|3|L|5|6|7|17|8|@$9|18|A|19|B|C]|$9|1A|A|1B|B|C]|$9|1C|A|1D|B|C]|$9|1E|A|1F|B|C]|$9|1G|A|1H|B|C]|$9|1I|A|1J|B|C]]|D|@]|E|$]]|$1|M|3|-4|5|6|7|1K|8|@]|D|@]|E|$]]]|N|$O|$5|P|Q|R|E|$S|T]]]]

Given the comment on <a href="https://stackoverflow.com/questions/3637377/does-this-method-violate-solid-or-has-code-smell/3637404#3637404">this answer</a> from the OP, I think the best approach would be to create a custom container class to replace <code>IList&lt;IDisplayable&gt; displayableItems</code> which has methods like <code>containsHumans()</code> and <code>containsAnimals()</code> so you can encapsulate the icky non-polymorphic code in one place and keep the logic in your <code>Summarize()</code> function clean.

<pre><code>class MyCollection : List&lt;IDisplayable&gt;
{
 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() &amp;&amp; !displayableItems.containsAnimals())
 {
 // do human-only logic here
 }
 else if (!displayableItems.containsHumans() &amp;&amp; displayableItems.containsAnimals())
 {
 // do animal-only logic here
 }
 else
 {
 // do logic for both here
 }

 return summary.ToString();
}
</code></pre>

Of course my example is overly simple and contrived. For instance, either as part of the logic in your <code>Summarize()</code> if/else statements, or perhaps surrounding the entire block, you'll want to iterate over the <code>displayableItems</code> collection. Also, you'll likely get better performance if you override <code>Add()</code> and <code>Remove()</code> in <code>MyCollection</code> and have them check the type of the object and set a flag, so your <code>containsHumans()</code> function (and others) can simply return the state of the flag and not have to iterate the collection every time they're called.

Does this give any code smell or violate SOLID principles?

<pre><code>public string Summarize()
{
IList&lt;IDisplayable&gt; 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();
}
</code></pre>

As you can see, my Summarize() is tied to implementation classes such as Human, Animal, etc. 

Is this code violating LSP? (Any other SOLID principles?)

Tying a method to implementation classes

翻译质量差，导致语言生硬或混乱。

没有提供实际的解决方法或示例。

解答不清晰，无法理解或解决问题。

页面排版不美观，阅读体验差。

文章

问答

视频

学习中心

腾讯云实验室

直播

竞赛

腾讯云代码分析专区

腾讯iOA零信任安全管理系统专区

腾讯云架构师技术同盟交流圈

腾讯云数据库专区

腾讯云顾问专区

腾讯云原生专区

腾讯混元专区

腾讯云TCE专区

腾讯云Lighthouse专区

腾讯云HAI专区

腾讯云Edgeone专区

腾讯云存储专区

腾讯云智能专区

腾讯轻联专区 

腾讯云开发专区

TAPD专区

腾讯轻量云游戏服专区

腾讯云最具价值专家

腾讯云架构师技术同盟

腾讯云创作之星

腾讯云开发者先锋

腾讯云代码助手

云原生构建

TAPD 敏捷项目管理

Cloud Studio

SDK中心

API中心

命令行工具

涵盖代码开发、场景应用、自动测试全流程，助你从零构建专属AI助手

一站式MCP教程库，解锁AI应用新玩法

这是否给代码带来了异味或违反了SOLID原则？public string Summarize(){IList<IDisplayable> displayableItems = getAllDisplayableItems();StringBuilder summary = new StringBuilder();for...

问将方法绑定到实现类
EN

Stack Overflow用户

社区

活动

圈层

关于

腾讯云开发者

热门产品

热门推荐

更多推荐

问将方法绑定到实现类EN