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

Question

Дает ли это какой-либо запах кода или нарушает принципы 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? (Есть ли другие твердые принципы?)

Answer 1

Я что-то чую ...

Если все ваши классы реализуют 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
}

Затем вы можете изменить свой цикл на что-то более чистое (и позволить классам реализовать собственную логику отображения):

Answer 2

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

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

Answer 3

Да.

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

С учетом комментария к этого ответа из 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() (и другие) могла просто возвращать состояние флага и нет необходимости повторять коллекцию каждый раз, когда они вызываются.

Answer 5

Как минимум, это нарушает LSP и принцип Open-Closed.

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

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

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

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

Answer 6

Как насчет:

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

Answer 7

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