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

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

Blog on Medium

I started an English-language blog on the Medium. I will be glad if you join https://medium.com/@alexander.byndyu

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

Принцип единственности ответственности

Формулировка: не должно быть больше одной причины для изменения класса

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

Примеры

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

1. Active Record

Проблема

Совсем недавно в качестве ORM я использовал MyGeneration. Суть этой ORM состоит в том, что по таблицам базы данных, она генерирует бизнес-сущности. Возьмем для примера сущность пользователя - Account. Сценарий использования выглядит так:

   1:  // создание пользователя
   2:  Accounts account = new Accounts();
   3:  account.AddNew();
   4:  account.Name = "Name";
   5:  account.Save();
   6:   
   7:  // загрузка объекта по Id
   8:  Accounts account = new Accounts()
   9:  account.LoadByPrimaryKey(1);
  10:   
  11:  // загрузка связной коллекции при обращении к свойству объекта
  12:  var list = account.Roles;

Шаблон Active Record может быть успешно использован в небольших проектах с простой бизнес-логикой. Практика показывает, что когда проект разрастается, то из-за смешанной логики внутри доменных объектов возникает много дублирования в коде и непредвиденных ошибок. Обращения к базе данных довольно сложно проследить, когда они скрыты, например, за свойством объекта account.Roles.

В данном случае объект Account имеет несколько ответственностей:

  1. является объектом домена и хранит бизнес-правила, например, связь с коллекцией ролей
  2. является точкой доступа к базе данных

Решение

Простым и действенным выходом является использование шаблона Repository. Хранилищу AccountRepository мы оставляем работу с базой данных и получаем «чистый» доменный объект.

   1:  // создание пользователя
   2:  var account = new Account();
   3:  account.Name = "Name";
   4:  accountRepository.Save(account);
   5:   
   6:  // загрузка пользователя по Id
   7:  var account = accountRepository.GetById(1);
   8:   
   9:  // загрузка со связной коллекцией
  10:  // пример из LLBLGen Pro
  11:  var account = accountRepository.GetById(1, new IPath[]{new Path<Account>(Account.PrefetchPathRoles)});

2. Валидация данных

Проблема

Если вы сделали хотя бы один проект, то перед вами наверняка стояла проблема валидации данных. Например, проверка введенного адреса эл. почты, длины имени пользователя, сложности пароля и т.п. Для валидации объекта резонно возникает первая реализация:

   1:  public class Product
   2:  {
   3:      public int Price { get; set; }
   4:   
   5:      public bool IsValid()
   6:      {
   7:          return Price > 0;
   8:      }
   9:  }
  10:   
  11:  // проверка на валидность
  12:  var product = new Product { Price = 100 };
  13:  var isValid = product.IsValid();

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

Теперь наш объект Product начал использовать в некоем CustomerService, который считает валидным продукт с ценой больше 100 тыс. рублей. Что делать? Уже сейчас понятно, что нам придется изменять наш объект продукта, например, таким образом:

   1:  public class Product
   2:  {
   3:      public int Price { get; set; }
   4:   
   5:      public bool IsValid(bool isCustomerService)
   6:      {
   7:          if (isCustomerService == true)
   8:              return Price > 100000;
   9:   
  10:          return Price > 0;
  11:      }
  12:  }
  13:   
  14:  // используем объект продукта в новом сервисе
  15:  var product = new Product { Price = 100 };
  16:  var isValid = product.IsValid(true);

Решение

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

   1:  public interface IProductValidator
   2:  {
   3:      bool IsValid(Product product);
   4:  }
   5:   
   6:  public class ProductDefaultValidator : IProductValidator
   7:  {
   8:      public bool IsValid(Product product)
   9:      {
  10:          return product.Price > 0;
  11:      }
  12:  }
  13:   
  14:  public class CustomerServiceProductValidator : IProductValidator
  15:  {
  16:      public bool IsValid(Product product)
  17:      {
  18:          return product.Price > 100000;
  19:      }
  20:  }
  21:   
  22:  public class Product
  23:  {
  24:      private readonly IProductValidator validator;
  25:   
  26:      public Product() : this(new ProductDefaultValidator())
  27:      {
  28:      }
  29:   
  30:      public Product(IProductValidator validator)
  31:      {
  32:          this.validator = validator;
  33:      }
  34:   
  35:      public int Price { get; set; }
  36:   
  37:      public bool IsValid()
  38:      {
  39:          return validator.IsValid(this);
  40:      }
  41:  }
  42:   
  43:  // обычное использование
  44:  var product = new Product { Price = 100 };
  45:   
  46:  // используем объект продукта в новом сервисе
  47:  var product = new Product (new CustomerServiceProductValidator()) { Price = 100 };

Имеем объект Product отдельно, а любое количество всяческих валидаторов отдельно.

В дополнение хочу посоветовать книгу Применение DDD и шаблонов проектирования. Проблемно-ориентированное проектирование приложений с примерами на C# и .NET. В ней очень подробно рассмотрен вопрос валидации данных.

3. God object

Проблема

Предел нарушения принципа единственности ответственности – God object. Этот объект знает и умеет делать все, что только можно. Например, он делает запросы к базе данных, к файловой системе, общается по протоколам в сеть и содержить тонну бизнес-логики. В пример приведу объект, который называется ImageHelper:

   1:  public static class ImageHelper
   2:  {
   3:      public static void Save(Image image)
   4:      {
   5:          // сохранение изображение в файловой системе
   6:      }
   7:   
   8:      public static int DeleteDuplicates()
   9:      {
  10:          // удалить из файловой системы все дублирующиеся изображения и вернуть количество удаленных
  11:      }
  12:   
  13:      public static Image SetImageAsAccountPicture(Image image, Account account)
  14:      {
  15:          // запрос к базе данных для сохранения ссылки на это изображение для пользователя
  16:      }
  17:   
  18:      public static Image Resize(Image image, int height, int width)
  19:      {
  20:          // изменение размеров изображения
  21:      }
  22:   
  23:      public static Image InvertColors(Image image)
  24:      {
  25:          // изменить цвета на изображении
  26:      }
  27:   
  28:      public static byte[] Download(Url imageUrl)
  29:      {
  30:          // загрузка битового массива с изображением с помощью HTTP запроса
  31:      }
  32:   
  33:      // и т.п.
  34:  }

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

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

Решение

Решением является разделить этот класс по принципу единственности ответственности: один класс на одну ответственность.

   1:  public static class ImageFileManager
   2:  {
   3:      public static void Save(Image image)
   4:      {
   5:          // сохранение изображение в файловой системе
   6:      }
   7:   
   8:      public static int DeleteDuplicates()
   9:      {
  10:          // удалить из файловой системы все дублирующиеся изображения и вернуть количество удаленных
  11:      }
  12:  }
  13:   
  14:  public static class ImageRepository
  15:  {
  16:      public static Image SetImageAsAccountPicture(Image image, Account account)
  17:      {
  18:          // запрос к базе данных для сохранения ссылки на это изображение для пользователя
  19:      }
  20:  }
  21:   
  22:  public static class Graphics
  23:  {
  24:   
  25:      public static Image Resize(Image image, int height, int width)
  26:      {
  27:          // изменение размеров изображения
  28:      }
  29:   
  30:      public static Image InvertColors(Image image)
  31:      {
  32:          // изменить цвета на изображении
  33:      }
  34:  }
  35:   
  36:   
  37:  public static class ImageHttpManager
  38:  {
  39:      public static byte[] Download(Url imageUrl)
  40:      {
  41:          // загрузка битового массива с изображением с помощью HTTP запроса
  42:      }
  43:  }

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


Ссылки

Глава «The Single Responsibility Principle» из книги Роберта Мартина «Agile Principles, Patterns, and Practices in C#»

InfoQ: Making Roles Explicit

LosTechies: Single Responsibility Principle

CodeBetter: Single Responsibility Principle

DimeCasts: Creating SOLID Code: Single Responsibility Principle

33 комментария:

  1. Респект аФФтору. Жду новых статей.

    ОтветитьУдалить
  2. Спасибо, статья понравилась, очень классные примеры
    жду продолжения

    ОтветитьУдалить
  3. Хорошие примеры. Замечу, что в примере с валидацией объекту Product лучше вообще не знать, что его валидируют. Это позволит пропустить его через несколько валидаторов.

    ОтветитьУдалить
  4. @Андрей
    Да, можно еще передавать список валидаторов

    http://stackoverflow.com/questions/409495/how-to-separate-data-validation-from-my-simple-domain-objects-pocos

    Или хранить список внутри объекта и инжектировать его с помощью DI контейнера. В общем, вариаций много, надо выбирать работающую в конкретной ситуации.

    ОтветитьУдалить
  5. А еще лучше использоват Composite (GoF)

    public class CompositeProductValidator : IProductValidator
    {
    private readonly IEnumerable{IProductValidator} validators;

    public CompositeProductValidator(IEnumerable{IProductValidator} validators) { this.validators = validators; }

    public bool IsValid(Product product) { return validators.All(v => v.IsValid(product)); }
    }

    Это позволит применять коллекцию валидаторов, не нарушая текущую структуру класса Product :)

    ОтветитьУдалить
  6. >God object
    >Решение: разбить класс на несколько..
    На этом этапе все понятно, но если есть класс, который в конструкторе получал этот самый God object, то что он будет получать теперь? Список аргументов в кол-ве равном кол-ву классов, на которые распался God? Поскольку одному God'у известно на сколько классов он может разложиться, то такой список параметров конструкора может стать весьма внушительным и неудобным.
    Как быть в таком случае?

    ОтветитьУдалить
  7. @silverlight
    Это статический класс и в конструктор его передать нельзя.
    Но допустим, что он не статический. Я так понял, что вопрос о клиентах этого "большого" класса.

    Если есть только один объект, который использует _все_ методы нашего god-object, значит проблемы нет. Рефакторингом будет разбивать эти объекты параллельно на несколько классов.

    Если есть несколько объектов, которые пользуются нашим god-object, значит наверняка каждому из них нужен один или несколько методов god-object. Т.е. они не знают об остальных методах. В таком случае надо выделить интерфейсы из god-object такие, чтобы каждый клиент использовал свой набор функций.

    Было:

    public class GodObjectClient
    {
    public void SaveImage(Image image)
    {
    ImageHelper.Save(image);
    }
    }

    Стало:

    public interface ISaveImage
    {
    void Save(Image image);
    }

    public class GodObjectClient
    {
    private readonly ISaveImage imageSaver;

    public GodObjectClient(ISaveImage imageSaver)
    {
    this.imageSaver = imageSaver;
    }

    public void SaveImage(Image image)
    {
    imageSaver.Save(image);
    }
    }

    Этот вопрос лежит в плоскости принципа разделения интерфейса, я сейчас как раз заканчиваю работу над постом по этой теме. Думаю в конце недели вы получите более развернутый ответ ;)

    ОтветитьУдалить
  8. Проглядел, что статический, но вы поняли правильно. Давайте немного изменим условия: тот же сюжет с GodObject, который передается в качестве параметра в класс A, но класс A не пользуется им непосредственно, а передает в объекты B, C, D, которые создаются в классе A. Каждому из них действительно нужны только некоторые методы GodObject, но протягивать его в раздробленном виде через класс A очень неудобно. Хотелось бы услышать ваши комментарии.

    ОтветитьУдалить
  9. @Sergey
    Лучше напишите код, иначе будем ходить кругами =)

    ОтветитьУдалить
  10. @Sergey

    Возможно этот пример даст ответ http://blog.byndyu.ru/2009/11/blog-post_19.html#fat-interface

    ОтветитьУдалить
  11. Думаю, что привести код - не лучший вариант, т.к. код большой, к тому же код - это не просто код, а еще и логика, которая окажется оторвана от целого. Про разделение интерфейса прочел, в общем-то проблема не в том как разделить, а в том, что потом придется тащить вместо одного объекта целую тучу параметров. Скорее всего у меня хромает проектирование где-то вдругом месте, что сказывается на моменте, котрый я пытался описать.

    ОтветитьУдалить
  12. Я бы хотел несколько отстраниться от вопросов, которые задал ранее, и задать еще один :)

    Скажем есть схема порождения объектов классов: A->B->C->D.
    Для создания объекта класса D требуется экземпляр класса X. Причем в классах A,B,C экземпляр X совершенно не нужен. Что в таких случаях делают опытные проектировщики, тащат X через A,B,C или тут изъян в чем-то другом?

    ОтветитьУдалить
  13. @Sergey
    Давайте не решать задачу про сферического коня в вакууме. Все-таки пришлите мне код и тогда можно будет увидеть в чем проблема.

    А вообще довольно сложно представить себе, что в родительском классе D нужен объект X, а в наследниках нет.

    ОтветитьУдалить
  14. Как же быть с моделью в паттерне mvc?

    ОтветитьУдалить
  15. А можно подробнее про загрузку зависимостей? LLBLGen к сожалению не знаю.

    ОтветитьУдалить
  16. @OgO
    Можно уточнить вопрос? Загрузка зависимостей - это то, как происходит инжектирование реальных объектов вместо интерфейсов? Или это про использование IoC контейнеров?

    На всякий случай уточню, что LLBLGen - это ORM.

    ОтветитьУдалить
  17. Я имею ввиду зависимости по предметной области, связанные коллекции, в вашем примере роли у пользователя. Мне интересен смысл второго параметра в данной строчке:
    var account = accountRepository.GetById(1, new IPath[ ]{new Path(Account.PrefetchPathRoles)});

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

    Может быть иногда лучше просто создать интерфейс GodObject и пользоваться им а не разделять класс на кучу других классов.( Почему то сразу подумал о Заместитель (Proxy) )

    ОтветитьУдалить
  19. @archibald74

    > Получается что классы начнут разростатся как грибы

    Что в этом плохого? Если они понятно называются, сгруппированы по функциональности, то проблем никаких не возникает.

    Лучше 3 понятных класса, чем один, делающий всё сразу.

    > А что если данный класс носит библиотечный характер, ну просто содержит список методов занимающихся одним действием ну скажем арифметикой один метод складывает другой вычитает третий умножает и т.д.

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

    > Может быть иногда лучше просто создать интерфейс GodObject

    В некоторых случаях да и это называется Фасад.

    ОтветитьУдалить
  20. Интерфейс определяет контракт (ответственности) . получается если имплементить более чем один интерфейс, будет нарушен принцип единственной ответственности :))

    ОтветитьУдалить
  21. @Faust
    Не всегда так... Есть интерфейсы, которые относятся к бизнесу, а есть, которые относятся к техническим деталям использования. Если я имплеменчу, скажем, IDisposible и IMyInterface, не думаю, что здесь нарушается принцип единственности ответственности.

    ОтветитьУдалить
  22. @Александр

    Шикарно, полностью согласен.

    Есть варианты, как провести черту между "бизнес-интерфейсами" и техническими?

    ОтветитьУдалить
  23. @Александр Бындю
    что-то вроде "что должен делать класс" и "как он должен это делать" )))

    ОтветитьУдалить
  24. @Faust
    Хотел бы заметить, что ваше утверждение не всегда верно. Имплементирование только одного, но неудачно спроектированного интерфейса может нарушить принцип единой ответственности. На основе примера, приведенного выше:

    public interface IImageHelper
    {
    void Save(Image image)
    int DeleteDuplicates()
    Image SetImageAsAccountPicture(Image image, Account account)
    Image Resize(Image image, int height, int width)
    Image InvertColors(Image image)
    byte[] Download(Url imageUrl)
    }

    ОтветитьУдалить
  25. @Александр Бындю

    Скорее всего, по принципу принадлежности/непринадлежности к предметной области.
    Принадлежность интерфейса к самому фреймворку (IDisposable, IEnumerable...), то есть его сервисная направленность может достоверно служить признаком такой "техничности"(?)

    ОтветитьУдалить
  26. IoC контейнер сам должен разобраться, где взять для класса D нужный ему X,
    т.е. созавая объект A автоматом создастся вся цепочка (B->C->D), и IoC контейнер разрулит все зависимосте ниже. 

    ОтветитьУдалить
  27. Александр, прежде всего, спасибо за блог, отличная работа! Последние пару дней посвятил изучению статей об ioc и tdd. На данный момент столкнулся с проблемой, которую применительно к вашему примеру с валидацией данных можно сформулировать так: в случае неудачной проверки Product::isValid в слое представления необходимо вывести сообщение пользователю с подробной информацией. Проблема в том, что часть этой информации хранится в конкретном валидаторе, а Product имеет только ссылку на интерфейс. Например, нужно вывести "Невозможно сохранить продукт, т.к. цена больше 100000", но 100000 содержатся в конкретном (CustomerServiceProductValidator) валидаторе.

    ОтветитьУдалить
  28. Возможно вам поможет вот это видео 
    http://www.infoq.com/presentations/Making-Roles-Explicit-Udi-Dahan

    ОтветитьУдалить
  29. Если у меня есть ImageHelper и он исполняет роль Facad`a, - не содержит логики, а только вызывает методы других класов и имеет четкий интерфейс, то это вполне допустимая реализация?

    ОтветитьУдалить
  30. Я думаю, что стоит посмотреть на этот класс. Надо понимать цель его существования :) Если можно, то хотелось бы увидеть код.

    ОтветитьУдалить