Phương pháp này có vi phạm RẮN hay có mùi mã không?

Question

Điều này có cho bất kỳ mã nào có mùi hoặc vi phạm nguyên tắc SOLID không?

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

Như bạn thấy, Tóm tắt của tôi() được gắn với các lớp học thực hiện như con người, động vật, vv

Liệu nó có mùi? Tôi có vi phạm LSP không? Bất kỳ nguyên tắc SOLID nào khác?

Answer 1

Với những nhận xét trên this answer từ OP nhiều, tôi nghĩ rằng cách tiếp cận tốt nhất là để tạo ra một lớp container tùy ý thay thế IList<IDisplayable> displayableItems trong đó có phương pháp như containsHumans() và containsAnimals() để bạn có thể đóng gói các phi icky mã đa hình ở một nơi và giữ logic trong hàm Summarize() của bạn sạch sẽ.

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

Tất nhiên, ví dụ của tôi quá đơn giản và không bị xâm phạm.Ví dụ, hoặc là một phần của logic trong Summarize() câu lệnh if/else của bạn, hoặc có lẽ xung quanh toàn bộ khối, bạn sẽ muốn lặp qua bộ sưu tập displayableItems. Ngoài ra, bạn có thể sẽ có hiệu suất tốt hơn nếu bạn ghi đè lên Add() và Remove() trong MyCollection và yêu cầu họ kiểm tra loại đối tượng và đặt cờ, vì vậy chức năng containsHumans() của bạn (và những thứ khác) có thể đơn giản trả về trạng thái của cờ và không có để lặp lại bộ sưu tập mỗi khi chúng được gọi.

Answer 2

Tôi ngửi thấy một chút gì đó ...

Nếu tất cả các lớp của bạn thực thi IDisplayable, mỗi người nên thực hiện logic riêng của mình để tự hiển thị. Bằng cách đó vòng lặp của bạn sẽ thay đổi một cái gì đó nhiều bụi:

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 
} 

Sau đó, bạn có thể thay đổi vòng lặp của bạn để một cái gì đó sạch hơn nhiều (và cho phép các lớp học để thực hiện logic hiển thị của riêng mình):

foreach(IDisplayable item in displayableItems) 
    summary.Append(item.Display()); 

Answer 3

có vẻ như IDisplayable nên có một phương pháp cho tên hiển thị để bạn có thể giảm bớt phương pháp mà một cái gì đó giống như

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

Answer 4

Làm thế nào về:

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

Answer 5

Có.

Tại sao không phải mỗi lớp thực hiện một phương pháp từ IDisplayable cho thấy loại của họ:

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

Sau đó chỉ cần gọi phương thức của bạn như sau:

foreach(IDisplayable item in displayableItems) 
{ 
    Console.WriteLine(item.GetInfo()); 
} 

Answer 6

Tối thiểu vi phạm LSP và Open- Nguyên tắc đóng.

Giải pháp là phải có một tài sản để giao diện IDisplayableDescription, vì vậy Tóm tắt mà chỉ có thể gọi

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

này cũng có thể được giải quyết thông qua phản ánh, vì bạn chỉ nhận được tên của lớp.

Giải pháp tốt hơn là trả lại thứ gì đó như là IDisplayableInfo từ phương thức GetInfo(). Đây sẽ là một điểm mở rộng để giúp bảo toàn OCP.

Answer 7

Nếu bạn không thể sửa đổi IDisplayable hoặc triển khai lớp và bạn đang sử dụng .NET 3.5 hoặc mới hơn, bạn có thể sử dụng phương pháp mở rộng. Nhưng đó không phải là tốt hơn