Формулировка: клиенты не должны зависеть от методов, которые они не используют
Как и при использовании других принципов проектирования классов мы пытаемся избавиться от ненужных зависимостей в коде, сделать код легко читаемым и легко изменяемым.
Примеры
Лишняя абстракция в наследовании
Проблема
Речь идет о базовых классах, которые вынуждают своих наследников знать и делать слишком много. Печально известный пример – класс MembershipProvider. Для использования этого класса нужно реализовать 27 абстрактных методов и свойств.
Рассмотрим пример из жизни. Это, конечно, не скопированный код один в один, но очень приближено к тому, что было. Есть базовый класс для аудиторов EntityAuditor. Он унаследован от класса AuditorBase, который предоставляет ORM, и реализует метод AuditEntityFieldSet этого базового класса. Также EntityAuditor добавляет свой абстрактный метод CreateLogRow, который используется в методе AuditEntityFieldSet и должен быть переопределен в конкретных реализациях:
public abstract class EntityAuditor : AuditorBase
{
public override void AuditEntityFieldSet(IEntityCore entity, int fieldIndex, object originalValue)
{
// ...
CreateLogRow(...
// ...
}
protected abstract LogRowEntity CreateLogRow(decimal? fieldId, string oldValue, string newValue, IEntityCore entity);
}
После этого начинаем реализовывать наследников. Например, создадим аудитор для класса Product:
public class ProductAuditor : EntityAuditor
{
protected override LogRowEntity CreateLogRow(decimal? fieldId, string oldValue, string newValue, IEntityCore entity)
{
// ...
}
}
Сейчас добавлению наследников ничего не мешает. Теперь представим, что в методе AuditEntityFieldSet понадобилась дополнительная логика, при которой нужно вызвать метод UpdateDuplicates. Этот метод также является абстрактным и требует реализации в наследниках:
public abstract class EntityAuditor : AuditorBase
{
public override void AuditEntityFieldSet(IEntityCore entity, int fieldIndex, object originalValue)
{
// ...
CreateLogRow(...
UpdateDuplicates(...
// ...
}
protected abstract LogRowEntity CreateLogRow(decimal? fieldId, string oldValue, string newValue, IEntityCore entity);
protected abstract void UpdateDuplicates(IEntityCore entity, decimal fieldId, object current);
}
public class ProductAuditor : EntityAuditor
{
protected override LogRowEntity CreateLogRow(decimal? fieldId, string oldValue, string newValue, IEntityCore entity)
{
// ...
}
protected override void UpdateDuplicates(IEntityCore entity, decimal fieldId, object current)
{
// реализация
}
}
public class AccountAuditor : EntityAuditor
{
protected override LogRowEntity CreateLogRow(decimal? fieldId, string oldValue, string newValue, IEntityCore entity)
{
// ...
}
protected override void UpdateDuplicates(IEntityCore entity, decimal fieldId, object current)
{
// здесь ничего нет!
}
}
EntityAuditor требует реализации метода UpdateDuplicates даже в тех наследниках, где он не нужен, как, например, в AccountAuditor. Проблема в том, что частный случай (UpdateDuplicates), который используется только в половине наследников, мы сделали общим, т.е. обязательным для всех наследников нашего EntityAuditor. Получается, что чем больше наследников будет у EntityAuditor, тем больше бесполезного кода мы будем писать, тем больше наследники будут знать лишнего о своем базовом классе. Это может сильно помешать нам в дальнейшем при рефакторинге или изменении логики в EntityAuditor.
Решение
В данном случае решение очень простое. Если наследникам класса EntityAuditor не нужна функция UpdateDuplicates, то и реализовывать ее они не должны. В С# это делается простой заменой ключевого слова abstract на virtual:
public abstract class EntityAuditor : AuditorBase
{
public override void AuditEntityFieldSet(IEntityCore entity, int fieldIndex, object originalValue)
{
// ...
CreateLogRow(...
UpdateDuplicates(...
// ...
}
protected abstract LogRowEntity CreateLogRow(decimal? fieldId, string oldValue, string newValue, IEntityCore entity);
protected virtual void UpdateDuplicates(IEntityCore entity, decimal fieldId, object current)
{
}
}
Теперь наследники отчищены от ненужной связности:
public class ProductAuditor : EntityAuditor
{
protected override LogRowEntity CreateLogRow(decimal? fieldId, string oldValue, string newValue, IEntityCore entity)
{
// ...
}
protected override void UpdateDuplicates(IEntityCore entity, decimal fieldId, object current)
{
// реализация
}
}
public class AccountAuditor : EntityAuditor
{
protected override LogRowEntity CreateLogRow(decimal? fieldId, string oldValue, string newValue, IEntityCore entity)
{
// ...
}
}
В других случаях могут быть другие решения этой проблемы. Если у вас есть примеры из практики, давайте разбирать их в комментариях.
«Жирный» интерфейс
Проблема
У нас есть интерфейс ISpecification. С помощью него мы можем узнать подходит ли продукт заявке – метод IsSuitable и является ли поле продукта измененным – метод IsFieldChanged:
public interface ISpecification
{
bool IsSuitable(Product realty, Offer offer);
bool IsFieldChanged(Product oldValue, Product newValue);
}
Чем является наш модуль для сторонних клиентов? Он является набором интерфейсов, с помощью которых модуль может быть использован. В данном случае проблема заключается в том, что клиентом первой функции является консольное приложение, а второй – класс хранилища:
public interface ISpecification
{
bool IsSuitable(Product realty, Offer offer);
bool IsFieldChanged(Product oldValue, Product newValue);
}
///
/// Хранилище для продуктов
///
public class ProductRepository : IRepository
{
public void Save(Product product)
{
// ...
specification.IsFieldChanged(...
// ...
}
}
///
/// Программа расчета подходящих продуктов
///
public class Program
{
public static void Main(string[] args)
{
// ...
specification.IsSuitable(...
}
}
Допустим, что мы уже написали несколько конкретных спецификаций:
class PriceSpecification : ISpecification
{
public bool IsSuitable(Product realty, Offer offer)
{
// ...
}
public bool IsFieldChanged(Product oldValue, Product newValue)
{
// ...
}
}
...
Уже видно, что полученный результат нас не устраивает. При рефакторинге или изменинии логики в консольном приложении и методе IsSuitable, нам придется затронуть все классы, которые реализовали интерфейс ISpecification. Например, представьте, что будет если в метод IsSuitable мы захотим добавить еще один параметр? А если конкретных спецификаций накопилось уже с десяток? Основная мысль в том, что теперь различные части системы зависят друг от друга, хоть и косвенно. Консольное приложение зависит от логики хранилища и наоборот.
Решение
Главное правило в данном случае звучит так: если клиенты интерфейса разделены, то и интерфейс должен быть разделен соответствующим образом.
После разделения получаем:
public interface ISpecification
{
bool IsSuitable(Product realty, Offer offer);
}
public interface IChangeFieldDetector
{
bool IsFieldChanged(Product oldValue, Product newValue);
}
Теперь консольное приложение работает интерфейсом ISpecification, а хранилище работает с интерфейсом IChangeFieldDetector. Проблема с зависимостью решена.
Кроме этого, мы решили еще и проблему с наследниками первой реализации ISpecification. Теперь класс может реализовывать только один интерфейс, за счет чего его на много проще поддерживать:
class PriceSpecification : ISpecification
{
public bool IsSuitable(Product realty, Offer offer)
{
// ...
}
}
class PriceChangeFieldDetector : IChangeFieldDetector
{
public bool IsFieldChanged(Product oldValue, Product newValue)
{
// ...
}
}
Этот пост входит в серию Принципы проектирования классов (S.O.L.I.D.):
- Принцип единственности ответственности (The Single Responsibility Principle)
- Принцип открытости/закрытости (The Open Closed Principle)
- Принцип замещения Лисков (The Liskov Substitution Principle)
- Принцип разделения интерфейса (The Interface Segregation Principle)
- Принцип инверсии зависимости (The Dependency Inversion Principle)
Ссылки
Роберт Мартин: The Interface Segregation Principle

Меня смутило решение, предлагаемое тобой в главе "Лишняя абстракция в наследовании". Я вижу два недостатка у решения "заменить abstract на virtual":
ОтветитьУдалить1. Что делать, если UpdateDuplicates был бы частью интерфейса, а не абстрактного класса?
2. Наследники будут видеть метод, который им не нужен. Пусть это и не публичный интерфейс, но все равно нехорошо
Мне кажется более правильным решением было бы отнаследовать от EntityAuditor класс UpdatableEntityAuditor (условное название) и от него наследовать те Auditor'ы, которые нуждаются в обновлении дубликатов (другие же пусть наследуются от EntityAuditor).
Что скажешь?
@Idsa
ОтветитьУдалить> Что делать, если UpdateDuplicates был бы частью интерфейса, а не абстрактного класса?
Он же таковым не является.
> Наследники будут видеть метод, который им не нужен. Пусть это и не публичный интерфейс, но все равно нехорошо
Они его не переопределяют, это не значит, что он им не нужен.
> Мне кажется более правильным решением было бы отнаследовать от EntityAuditor...
Да, как вариант. Опять же зависит от конкретного случая. В нашем случае это был бы лишний класс.
>>Они его не переопределяют, это не значит, что он им не нужен.
ОтветитьУдалитьТогда очень странно, что в первоначальном дизайне функция была абстрактной.
Ну если они его не переопределяют, но он им нужен, то они получат ничего не делающий метод. Так нужен ли он им?
>>Да, как вариант. Опять же зависит от конкретного случая. В нашем случае это был бы лишний класс.
Согласен, но в рамках обучающего цикла статей мне видится более уместным пример, который встречается чаще на практике (чтобы читатель смог прочувствовать его на себе).
Твой пример показывает очень нетипичную ситуацию, и решение в виде пустого виртуального метода попахивает. А что если бы у метода UpdateDuplicates было возвращаемое значение? Как минимум, в том методе нужно выкидывать исключение.
@Idsa
ОтветитьУдалитьИдеальной системы не бывает, сначала она должна была быть абстрактной. В ходе появления других наследников стало видно, что это не так.
> Так нужен ли он им?
Зависит от того сколько будет других наследников, которым не нужен. Если много, то твой надо использовать твой подход.
> А что если бы у метода UpdateDuplicates было возвращаемое значение?
Всех вариантов просто невозможно перечислить. Для моей практики одни задачи и решения типичны, для твоей другие. Если хочешь напиши статью со своими вариантами и я добавлю на нее ссылку здесь.
>>Если хочешь напиши статью со своими вариантами и я добавлю на нее ссылку здесь.
ОтветитьУдалитьНе будем множить сущности. Думаю, этих комментариев будет вполне достаточно.
Попутно прочитал описание и примеры SOLID у Игоря Тамощука: http://igor.quatrocode.com/2008/09/solid-top-5.html (он в комментариях к одной из статей давал ссылку). И вот там принцип ISP демонстрируется способом, который я предлагал выше. Просто красота: ничего лишнего, не придраться. Хоть убей, не нравится мне твой пример: попахивает он. Я бы так не придирался, но твои статьи для многих являются отправной точкой в мир SOLID...
ОтветитьУдалить@Idsa
ОтветитьУдалитьОписание Игоря Тамощука - это мой второй пример. Но для меня было важно показать и первый случай, т.к. он довольно часто выявляется в коде.
К тому же у меня есть ссылки на первоисточники - статьи Роберта Мартина. Ты можешь найти ссылки на их в конце каждой моей статьи про SOLID.
Refuse bequest можно "рефаторить", как Replase inheritance with delegation. Рассмотрено у Даниэля Арсеновски в Рефаторинге.
ОтветитьУдалитьПо второму примеру не понятно: Если описание интерфейса IsSuitable таки изменится, все равно придется менять все классы которые его реализовали.
ОтветитьУдалить@Max
ОтветитьУдалить> Refuse bequest можно "рефаторить", как Replase inheritance with delegation.
Хороший вариант, спасибо.
> Если описание интерфейса IsSuitable таки изменится, все равно придется менять все классы которые его реализовали.
Да, классы менять в любом случае надо будет. Но только те, которые на самом деле должны использовать все методы интерфейса.
Здравствуйте.
ОтветитьУдалитьВот тут опечатка:"В данном случае проблема заключается в том, что клиентом первой функции является класс хранилища, а второй – консольное приложение:"
Первой (IsSuitable) - консольное, второй (IsFieldChanged) - хранилище
Спасибо, поправил.
ОтветитьУдалитьЕсли обратить внимание на документик в ссылках "Роберт Мартин: The Interface Segregation Principle", то автор использует аналогичный подход. Но в последнем абзаце он указывает, что такое решение идёт в расход с LSP.
ОтветитьУдалитьХотя стоп: это ж не метод интерфейса класса, это же защищённый шаблонный метод - при чём тут LSP =)
ОтветитьУдалитьНе думаю, что EntityAuditor должен вообще иметь метод UpdateDuplicates... Если уж такой метод и нужен, то только для личного использования (инкапсулировать этот метод в, реализующем интерфейс EntityAuditor, классе), что, как по-мне, может нарушить SRP и OCP. Может быть лучше вообще создать отдельный интерфейс DuplicatesUpdater?
ОтветитьУдалитьИгорь, в данный момент я бы создал два интерфейса под оба метода. Есть хорошая презентация от Udi Dahan на эту тему http://www.infoq.com/presentations/Making-Roles-Explicit-Udi-Dahan
ОтветитьУдалитьСогласен, хочется узнать мнение об этом методе, поскольку в нем я также вижу решение, но и откровенные серьезные недостатки. Сейчас я решаю эту задачу именно так, поскольку в языках, на которых сейчас работаю, нет механизма virtual в принципе, но...
ОтветитьУдалить