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