क्या यह विधि SOLID का उल्लंघन करती है या कोड गंध है?

Question

क्या यह कोई कोड गंध देता है या सोलिड सिद्धांतों का उल्लंघन करता है?

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(); 
} 

आप देख सकते हैं, मेरे संक्षेप में प्रस्तुत करना(), इस तरह के मानव, पशु के रूप में कार्यान्वयन वर्गों से जुड़ा हुआ है आदि

यह गंध है? क्या मैं एलएसपी का उल्लंघन कर रहा हूं? कोई अन्य ठोस सिद्धांत?

Answer 1

ओपी से 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(); 
} 

बेशक मेरा उदाहरण अत्यधिक सरल और समेकित है।उदाहरण के लिए, या तो Summarize() में तर्क के हिस्से के रूप में यदि/अन्य कथन, या शायद पूरे ब्लॉक के आस-पास, तो आप displayableItems संग्रह पर फिर से शुरू करना चाहेंगे। इसके अलावा, यदि आप Add() और Remove()MyCollection में ओवरराइड करते हैं तो आपको बेहतर प्रदर्शन मिलेगा और उन्हें ऑब्जेक्ट के प्रकार की जांच करें और ध्वज सेट करें, इसलिए आपका containsHumans() फ़ंक्शन (और अन्य) बस ध्वज की स्थिति वापस कर सकता है और नहीं जब भी उन्हें बुलाया जाता है तो संग्रह को फिर से शुरू करने के लिए।

Answer 2

मैं एक छोटे से कुछ ...

अपनी कक्षाओं सभी 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 3

की तरह लगता है IDisplayable प्रदर्शन नाम के लिए एक विधि होनी चाहिए ताकि आप की तरह

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

Answer 4

कुछ करने के लिए है कि विधि कम कर सकते हैं कैसे के बारे में:

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

Answer 5

हां।

क्यों नहीं हर वर्ग 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()); 
} 

Answer 6

एक न्यूनतम यह LSP और ओपन का उल्लंघन करती है पर बंद सिद्धांत। ताकि संक्षेप सिर्फ कॉल कर सकते हैं

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

यह भी, प्रतिबिंब के माध्यम से हल किया जा सकता है क्योंकि आप केवल क्लास के नाम हो रही है

समाधान, IDisplayable इंटरफ़ेस करने के लिए एक Description संपत्ति है।

बेहतर समाधान विधि से IDisplayableInfo जैसा कुछ वापस करना है। ओसीपी को संरक्षित रखने में मदद के लिए यह एक विस्तारशील बिंदु होगा।

Answer 7

यदि आप आईडीआईप्लेबल या क्लास कार्यान्वयन को संशोधित नहीं कर सकते हैं और आप .NET 3.5 का उपयोग कर रहे हैं या बाद में आप एक्सटेंशन विधियों का उपयोग कर सकते हैं। लेकिन उस कि ज्यादा बेहतर