9 октября 2009 г.

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

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

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

Примеры

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

1. Active Record

Проблема

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

// создание пользователя
Accounts account = new Accounts();
account.AddNew();
account.Name = "Name";
account.Save();
 
// загрузка объекта по Id
Accounts account = new Accounts()
account.LoadByPrimaryKey(1);
 
// загрузка связной коллекции при обращении к свойству объекта
var list = account.Roles;

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

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

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

Решение

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

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

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

Проблема

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

public class Product
{
    public int Price { get; set; }
 
    public bool IsValid()
    {
        return Price > 0;
    }
}
 
// проверка на валидность
var product = new Product { Price = 100 };
var isValid = product.IsValid();

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

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

public class Product
{
    public int Price { get; set; }
 
    public bool IsValid(bool isCustomerService)
    {
        if (isCustomerService == true)
            return Price > 100000;
 
        return Price > 0;
    }
}
 
// используем объект продукта в новом сервисе
var product = new Product { Price = 100 };
var isValid = product.IsValid(true);

Решение

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

public interface IProductValidator
{
    bool IsValid(Product product);
}
 
public class ProductDefaultValidator : IProductValidator
{
    public bool IsValid(Product product)
    {
        return product.Price > 0;
    }
}
 
public class CustomerServiceProductValidator : IProductValidator
{
    public bool IsValid(Product product)
    {
        return product.Price > 100000;
    }
}
 
public class Product
{
    private readonly IProductValidator validator;
 
    public Product() : this(new ProductDefaultValidator())
    {
    }
 
    public Product(IProductValidator validator)
    {
        this.validator = validator;
    }
 
    public int Price { get; set; }
 
    public bool IsValid()
    {
        return validator.IsValid(this);
    }
}
 
// обычное использование
var product = new Product { Price = 100 };
 
// используем объект продукта в новом сервисе
var product = new Product (new CustomerServiceProductValidator()) { Price = 100 };

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

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

3. God object

Проблема

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

public static class ImageHelper
{
    public static void Save(Image image)
    {
        // сохранение изображение в файловой системе
    }
 
    public static int DeleteDuplicates()
    {
        // удалить из файловой системы все дублирующиеся изображения и вернуть количество удаленных
    }
 
    public static Image SetImageAsAccountPicture(Image image, Account account)
    {
        // запрос к базе данных для сохранения ссылки на это изображение для пользователя
    }
 
    public static Image Resize(Image image, int height, int width)
    {
        // изменение размеров изображения
    }
 
    public static Image InvertColors(Image image)
    {
        // изменить цвета на изображении
    }
 
    public static byte[] Download(Url imageUrl)
    {
        // загрузка битового массива с изображением с помощью HTTP запроса
    }
 
    // и т.п.
}

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

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

Решение

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

public static class ImageFileManager
{
    public static void Save(Image image)
    {
        // сохранение изображение в файловой системе
    }
 
    public static int DeleteDuplicates()
    {
        // удалить из файловой системы все дублирующиеся изображения и вернуть количество удаленных
    }
}
 
public static class ImageRepository
{
    public static Image SetImageAsAccountPicture(Image image, Account account)
    {
        // запрос к базе данных для сохранения ссылки на это изображение для пользователя
    }
}
 
public static class Graphics
{
 
    public static Image Resize(Image image, int height, int width)
    {
        // изменение размеров изображения
    }
 
    public static Image InvertColors(Image image)
    {
        // изменить цвета на изображении
    }
}
 
public static class ImageHttpManager
{
    public static byte[] Download(Url imageUrl)
    {
        // загрузка битового массива с изображением с помощью HTTP запроса
    }
}

Этот пост входит в серию Принципы проектирования классов (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. Я думаю, что стоит посмотреть на этот класс. Надо понимать цель его существования :) Если можно, то хотелось бы увидеть код.

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