Обратная связь

Для меня очень важно получать обратную связь от Bас. Пишите мне, если есть вопросы, интересные темы для обсуждения или по любым другим поводам. Мой  почтовый ящик и гугл-группа.

пятница, 20 ноября 2009 г.

Принцип разделения интерфейса

Формулировка: клиенты не должны зависеть от методов, которые они не используют

Как и при использовании других принципов проектирования классов мы пытаемся избавиться от ненужных зависимостей в коде, сделать код легко читаемым и легко изменяемым.

Примеры

Лишняя абстракция в наследовании

Проблема

Речь идет о базовых классах, которые вынуждают своих наследников знать и делать слишком много. Печально известный пример – класс MembershipProvider. Для использования этого класса нужно реализовать 27 абстрактных методов и свойств.

Рассмотрим пример из жизни. Это, конечно, не скопированный код один в один, но очень приближено к тому, что было. Есть базовый класс для аудиторов EntityAuditor. Он унаследован от класса AuditorBase, который предоставляет ORM, и реализует метод AuditEntityFieldSet этого базового класса. Также EntityAuditor добавляет свой абстрактный метод CreateLogRow, который используется в методе AuditEntityFieldSet и должен быть переопределен в конкретных реализациях:

   1:  public abstract class EntityAuditor : AuditorBase
   2:  {
   3:      public override void AuditEntityFieldSet(IEntityCore entity, int fieldIndex, object originalValue)
   4:      {
   5:          // ...
   6:          
   7:          CreateLogRow(...
   8:          
   9:          // ...
  10:      }
  11:   
  12:      protected abstract LogRowEntity CreateLogRow(decimal? fieldId, string oldValue, string newValue, IEntityCore entity);
  13:  }

После этого начинаем реализовывать наследников. Например, создадим аудитор для класса Product:

   1:  public class ProductAuditor : EntityAuditor
   2:  {
   3:      protected override LogRowEntity CreateLogRow(decimal? fieldId, string oldValue, string newValue, IEntityCore entity)
   4:      {
   5:          // ...
   6:      }
   7:  }

Сейчас добавлению наследников ничего не мешает. Теперь представим, что в методе AuditEntityFieldSet понадобилась дополнительная логика, при которой нужно вызвать метод UpdateDuplicates. Этот метод также является абстрактным и требует реализации в наследниках:

   1:  public abstract class EntityAuditor : AuditorBase
   2:  {
   3:      public override void AuditEntityFieldSet(IEntityCore entity, int fieldIndex, object originalValue)
   4:      {
   5:          // ...
   6:          
   7:          CreateLogRow(...
   8:   
   9:          UpdateDuplicates(...
  10:          
  11:          // ...
  12:      }
  13:   
  14:      protected abstract LogRowEntity CreateLogRow(decimal? fieldId, string oldValue, string newValue, IEntityCore entity);
  15:   
  16:      protected abstract void UpdateDuplicates(IEntityCore entity, decimal fieldId, object current);
  17:  }
  18:   
  19:   
  20:  public class ProductAuditor : EntityAuditor
  21:  {
  22:      protected override LogRowEntity CreateLogRow(decimal? fieldId, string oldValue, string newValue, IEntityCore entity)
  23:      {
  24:          // ...
  25:      }
  26:   
  27:      protected override void UpdateDuplicates(IEntityCore entity, decimal fieldId, object current)
  28:      {
  29:          // реализация
  30:      }
  31:  }
  32:   
  33:  public class AccountAuditor : EntityAuditor
  34:  {
  35:      protected override LogRowEntity CreateLogRow(decimal? fieldId, string oldValue, string newValue, IEntityCore entity)
  36:      {
  37:          // ...
  38:      }
  39:   
  40:      protected override void UpdateDuplicates(IEntityCore entity, decimal fieldId, object current)
  41:      {
  42:          // здесь ничего нет!
  43:      }
  44:  }

EntityAuditor требует реализации метода UpdateDuplicates даже в тех наследниках, где он не нужен, как, например, в AccountAuditor. Проблема в том, что частный случай (UpdateDuplicates), который используется только в половине наследников, мы сделали общим, т.е. обязательным для всех наследников нашего EntityAuditor. Получается, что чем больше наследников будет у EntityAuditor, тем больше бесполезного кода мы будем писать, тем больше наследники будут знать лишнего о своем базовом классе. Это может сильно помешать нам в дальнейшем при рефакторинге или изменении логики в EntityAuditor.

Решение

В данном случае решение очень простое. Если наследникам класса EntityAuditor не нужна функция UpdateDuplicates, то и реализовывать ее они не должны. В С# это делается простой заменой ключевого слова abstract на virtual:

   1:  public abstract class EntityAuditor : AuditorBase
   2:  {
   3:      public override void AuditEntityFieldSet(IEntityCore entity, int fieldIndex, object originalValue)
   4:      {
   5:          // ...
   6:          
   7:          CreateLogRow(...
   8:   
   9:          UpdateDuplicates(...
  10:          
  11:          // ...
  12:      }
  13:   
  14:      protected abstract LogRowEntity CreateLogRow(decimal? fieldId, string oldValue, string newValue, IEntityCore entity);
  15:   
  16:      protected virtual void UpdateDuplicates(IEntityCore entity, decimal fieldId, object current)
  17:      {
  18:      }
  19:  }

Теперь наследники отчищены от ненужной связности:

   1:  public class ProductAuditor : EntityAuditor
   2:  {
   3:      protected override LogRowEntity CreateLogRow(decimal? fieldId, string oldValue, string newValue, IEntityCore entity)
   4:      {
   5:          // ...
   6:      }
   7:   
   8:      protected override void UpdateDuplicates(IEntityCore entity, decimal fieldId, object current)
   9:      {
  10:          // реализация
  11:      }
  12:  }
  13:   
  14:  public class AccountAuditor : EntityAuditor
  15:  {
  16:      protected override LogRowEntity CreateLogRow(decimal? fieldId, string oldValue, string newValue, IEntityCore entity)
  17:      {
  18:          // ...
  19:      }
  20:  }

В других случаях могут быть другие решения этой проблемы. Если у вас есть примеры из практики, давайте разбирать их в комментариях.

«Жирный» интерфейс

Проблема

У нас есть интерфейс ISpecification. С помощью него мы можем узнать подходит ли продукт заявке – метод IsSuitable и является ли поле продукта измененным – метод IsFieldChanged:

   1:  public interface ISpecification
   2:  {
   3:      bool IsSuitable(Product realty, Offer offer);
   4:      
   5:      bool IsFieldChanged(Product oldValue, Product newValue);
   6:  }

Чем является наш модуль для сторонних клиентов? Он является набором интерфейсов, с помощью которых модуль может быть использован. В данном случае проблема заключается в том, что клиентом первой функции является класс хранилища, а второй – консольное приложение:

   1:  public interface ISpecification
   2:  {
   3:      bool IsSuitable(Product realty, Offer offer);
   4:   
   5:      bool IsFieldChanged(Product oldValue, Product newValue);
   6:  }
   7:   
   8:  /// <summary>
   9:  /// Хранилище для продуктов
  10:  /// </summary>
  11:  public class ProductRepository : IRepository<Product>
  12:  {
  13:      public void Save(Product product)
  14:      {
  15:          // ...
  16:   
  17:          specification.IsFieldChanged(...
  18:   
  19:          // ...
  20:      }
  21:  }
  22:   
  23:  /// <summary>
  24:  /// Программа расчета подходящих продуктов
  25:  /// </summary>
  26:  public class Program
  27:  {
  28:      public static void Main(string[] args)
  29:      {
  30:          // ...
  31:   
  32:          specification.IsSuitable(...
  33:      }
  34:  }

Допустим, что мы уже написали несколько конкретных спецификаций:

   1:  class PriceSpecification : ISpecification
   2:  {
   3:      public bool IsSuitable(Product realty, Offer offer)
   4:      {
   5:          // ...
   6:      }
   7:   
   8:      public bool IsFieldChanged(Product oldValue, Product newValue)
   9:      {
  10:          // ...
  11:      }
  12:  }
  13:   
  14:  ...

Уже видно, что полученный результат нас не устраивает. При рефакторинге или изменинии логики в консольном приложении и методе IsSuitable, нам придется затронуть все классы, которые реализовали интерфейс ISpecification. Например, представьте, что будет если в метод IsSuitable мы захотим добавить еще один параметр? А если конкретных спецификаций накопилось уже с десяток? Основная мысль в том, что теперь различные части системы зависят друг от друга, хоть и косвенно. Консольное приложение зависит от логики хранилища и наоборот.

Решение

Главное правило в данном случае звучит так: если клиенты интерфейса разделены, то и интерфейс должен быть разделен соответствующим образом.

После разделения получаем:
   1:  public interface ISpecification
   2:  {
   3:      bool IsSuitable(Product realty, Offer offer);
   4:  }
   5:   
   6:  public interface IChangeFieldDetector
   7:  {
   8:      bool IsFieldChanged(Product oldValue, Product newValue);
   9:  }

Теперь консольное приложение работает интерфейсом ISpecification, а хранилище работает с интерфейсом IChangeFieldDetector. Проблема с зависимостью решена.

Кроме этого, мы решили еще и проблему с наследниками первой реализации ISpecification. Теперь класс может реализовывать только один интерфейс, за счет чего его на много проще поддерживать:

   1:  class PriceSpecification : ISpecification
   2:  {
   3:      public bool IsSuitable(Product realty, Offer offer)
   4:      {
   5:          // ...
   6:      }
   7:  }
   8:   
   9:  class PriceChangeFieldDetector : IChangeFieldDetector
  10:  {
  11:      public bool IsFieldChanged(Product oldValue, Product newValue)
  12:      {
  13:          // ...
  14:      }
  15:  }

Этот пост входит в серию Принципы проектирования классов (S.O.L.I.D.):


Ссылки

Роберт Мартин: The Interface Segregation Principle

DoodleProject: Interface Segregation Principle

LosTechies: The Interface Segregation Principle

10 коммент.:

  1. Меня смутило решение, предлагаемое тобой в главе "Лишняя абстракция в наследовании". Я вижу два недостатка у решения "заменить abstract на virtual":
    1. Что делать, если UpdateDuplicates был бы частью интерфейса, а не абстрактного класса?
    2. Наследники будут видеть метод, который им не нужен. Пусть это и не публичный интерфейс, но все равно нехорошо

    Мне кажется более правильным решением было бы отнаследовать от EntityAuditor класс UpdatableEntityAuditor (условное название) и от него наследовать те Auditor'ы, которые нуждаются в обновлении дубликатов (другие же пусть наследуются от EntityAuditor).

    Что скажешь?
    ОтветитьУдалить
  2. @Idsa
    > Что делать, если UpdateDuplicates был бы частью интерфейса, а не абстрактного класса?
    Он же таковым не является.

    > Наследники будут видеть метод, который им не нужен. Пусть это и не публичный интерфейс, но все равно нехорошо
    Они его не переопределяют, это не значит, что он им не нужен.

    > Мне кажется более правильным решением было бы отнаследовать от EntityAuditor...
    Да, как вариант. Опять же зависит от конкретного случая. В нашем случае это был бы лишний класс.
    ОтветитьУдалить
  3. >>Они его не переопределяют, это не значит, что он им не нужен.

    Тогда очень странно, что в первоначальном дизайне функция была абстрактной.

    Ну если они его не переопределяют, но он им нужен, то они получат ничего не делающий метод. Так нужен ли он им?

    >>Да, как вариант. Опять же зависит от конкретного случая. В нашем случае это был бы лишний класс.

    Согласен, но в рамках обучающего цикла статей мне видится более уместным пример, который встречается чаще на практике (чтобы читатель смог прочувствовать его на себе).

    Твой пример показывает очень нетипичную ситуацию, и решение в виде пустого виртуального метода попахивает. А что если бы у метода UpdateDuplicates было возвращаемое значение? Как минимум, в том методе нужно выкидывать исключение.
    ОтветитьУдалить
  4. @Idsa
    Идеальной системы не бывает, сначала она должна была быть абстрактной. В ходе появления других наследников стало видно, что это не так.

    > Так нужен ли он им?
    Зависит от того сколько будет других наследников, которым не нужен. Если много, то твой надо использовать твой подход.

    > А что если бы у метода UpdateDuplicates было возвращаемое значение?
    Всех вариантов просто невозможно перечислить. Для моей практики одни задачи и решения типичны, для твоей другие. Если хочешь напиши статью со своими вариантами и я добавлю на нее ссылку здесь.
    ОтветитьУдалить
  5. >>Если хочешь напиши статью со своими вариантами и я добавлю на нее ссылку здесь.

    Не будем множить сущности. Думаю, этих комментариев будет вполне достаточно.
    ОтветитьУдалить
  6. Попутно прочитал описание и примеры SOLID у Игоря Тамощука: http://igor.quatrocode.com/2008/09/solid-top-5.html (он в комментариях к одной из статей давал ссылку). И вот там принцип ISP демонстрируется способом, который я предлагал выше. Просто красота: ничего лишнего, не придраться. Хоть убей, не нравится мне твой пример: попахивает он. Я бы так не придирался, но твои статьи для многих являются отправной точкой в мир SOLID...
    ОтветитьУдалить
  7. @Idsa
    Описание Игоря Тамощука - это мой второй пример. Но для меня было важно показать и первый случай, т.к. он довольно часто выявляется в коде.

    К тому же у меня есть ссылки на первоисточники - статьи Роберта Мартина. Ты можешь найти ссылки на их в конце каждой моей статьи про SOLID.
    ОтветитьУдалить
  8. Refuse bequest можно "рефаторить", как Replase inheritance with delegation. Рассмотрено у Даниэля Арсеновски в Рефаторинге.
    ОтветитьУдалить
  9. По второму примеру не понятно: Если описание интерфейса IsSuitable таки изменится, все равно придется менять все классы которые его реализовали.
    ОтветитьУдалить
  10. @Max

    > Refuse bequest можно "рефаторить", как Replase inheritance with delegation.

    Хороший вариант, спасибо.

    > Если описание интерфейса IsSuitable таки изменится, все равно придется менять все классы которые его реализовали.

    Да, классы менять в любом случае надо будет. Но только те, которые на самом деле должны использовать все методы интерфейса.
    ОтветитьУдалить