Привязка метода к классам реализации

Дает ли это какой-либо запах или нарушает принципы 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();
}

Как видите, мой Summarize() привязан к классам реализации, таким как Human, Animal и т.д.

Этот код нарушает LSP? (Любые другие твердые принципы?)

Ответ 1

Учитывая комментарий этого ответа от OP, я думаю, что лучшим подходом было бы создание настраиваемого класса контейнера для замены 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() if/else, либо, возможно, вокруг всего блока, вы захотите выполнить итерацию по коллекции displayableItems. Кроме того, вы, скорее всего, получите лучшую производительность, если вы переопределите Add() и Remove() в MyCollection и попросите их проверить тип объекта и установить флаг, поэтому ваша функция containsHumans() (и другие) может просто вернуться состояние флага и не нужно перебирать коллекцию каждый раз, когда они вызываются.

Ответ 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());

Ответ 3

похоже, что у IDisplayable должен быть метод для отображаемого имени, чтобы вы могли уменьшить этот метод до чего-то вроде

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

Ответ 4

Да.

Почему бы не каждый класс реализовать метод из 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());
}

Ответ 5

Как насчет:

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

Ответ 6

Как минимум, это нарушает LSP и открытый принцип.

Решение состоит в том, чтобы иметь свойство Description для интерфейса IDisplayable, так что суммирование может просто вызвать

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

Это также можно решить путем отражения, поскольку вы получаете только имя класса.

Лучшее решение - вернуть что-то вроде IDisplayableInfo из метода GetInfo(). Это будет точкой расширяемости, чтобы помочь сохранить OCP.

Ответ 7

Если вы не можете изменить IDisplayable или реализации класса, и вы используете .NET 3.5 или более позднюю версию, вы можете использовать методы расширения. Но это не намного лучше