此方法是否違反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或有代碼味道？

回答

相關問題