Этот класс дизайн нарушает принцип единой ответственности?

Сегодня у меня был спор с кем-то.

Я объяснял преимущества наличия модели богатых доменов по сравнению с моделью анемичных доменов. И я продемонстрировал свою точку зрения с помощью простого класса, который выглядит так:

public class Employee
{
    public Employee(string firstName, string lastName)
    {
        FirstName = firstName;
        LastName = lastname;
    }

    public string FirstName { get private set; }
    public string LastName { get; private set;}
    public int CountPaidDaysOffGranted { get; private set;}

    public void AddPaidDaysOffGranted(int numberOfdays)
    {
        // Do stuff
    }
}

Когда он защищал свой анемичный модельный подход, одним из его аргументов было: «Я верю в SOLID . Вы нарушаете принцип единой ответственности (SRP), поскольку вы одновременно представляете данные и выполняете логику в одном классе».

Я нахожу это утверждение действительно удивительным, поскольку, следуя этим рассуждениям, любой класс, имеющий одно свойство и один метод, нарушает SRP, и поэтому ООП в целом не является ТВЕРДЫМ, а функциональное программирование - единственный путь в рай.

Я решил не отвечать на его многочисленные аргументы, но мне любопытно, что сообщество думает по этому вопросу.

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

Что бы вы сказали?

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

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

Ваш коллега фокусируется не на том. Вопрос не в том, "какие члены у этого класса?" но "какие полезные операции выполняет этот класс в программе?" Как только это будет понято, модель вашего домена выглядит просто отлично.

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

Поскольку этот класс имеет дело только с Employee, я думаю, будет справедливо сказать, что он не нарушает SRP, но у людей всегда будет свое мнение.

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

На мой взгляд, этот класс может потенциально нарушать SRP, если он будет продолжать представлять Employeeи EmployeeHolidays.

Как бы то ни было, и если бы оно пришло ко мне на экспертную оценку, я бы, наверное, пропустил это. Если бы было добавлено больше специфических для Сотрудника свойств и методов и добавлено больше специфических для праздника свойств, я бы, вероятно, посоветовал разделить, сославшись на SRP и ISP.

Уже есть отличные ответы, указывающие на то, что SRP - это абстрактное понятие о логической функциональности, но есть тонкие моменты, которые, я думаю, стоит добавить.

Первые две буквы в SOLID, SRP и OCP - это то, как ваш код изменяется в ответ на изменение требований. Мое любимое определение SRP: «модуль / класс / функция должен иметь только одну причину для изменения». Спорить о вероятных причинах изменения вашего кода гораздо продуктивнее, чем спорить о том, является ли ваш код твердым или нет.

Сколько причин должен изменить ваш класс Сотрудника? Я не знаю, потому что я не знаю контекст, в котором вы его используете, и я также не вижу будущего. Что я могу сделать, так это провести мозговой штурм возможных изменений на основе того, что я видел в прошлом, и вы можете субъективно оценить, насколько они вероятны. Если между «разумно вероятным» и «моим кодом по той же причине уже более одного балла», то вы нарушаете SRP против такого рода изменений. Вот один из них: кто-то с более чем двумя именами присоединяется к вашей компании (или архитектор читает эту прекрасную статью о W3C ). Вот еще: ваша компания меняет то, как она распределяет праздничные дни.

Обратите внимание, что эти причины одинаково действительны, даже если вы удалите метод AddHolidays. Множество моделей анемичных доменов нарушают SRP. Многие из них являются просто таблицами базы данных в коде, и для таблиц базы данных очень часто бывает более 20 причин для изменения.

Вот что нужно пережить: изменится ли ваш класс Employee, если вашей системе потребуется отслеживать зарплаты сотрудников? Адреса? Экстренная контактная информация? Если вы скажете «да» (и «вероятно, произойдет») двум из них, то ваш класс будет нарушать SRP, даже если в нем еще нет кода! SOLID касается процессов и архитектуры в той же мере, что и кода.

Ответственность за то, что класс представляет данные, не является частной реализацией.

У класса есть одна обязанность - представлять работника. В этом контексте это означает, что он представляет некоторый общедоступный API, который предоставляет вам функциональность, необходимую для работы с сотрудниками (является ли хорошим примером пример AddHolidays).

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

Идея о том, что логика и данные каким-либо образом всегда ошибочны, настолько нелепа, что даже не заслуживает обсуждения. Однако в этом примере действительно есть явное нарушение единственной ответственности, но это не потому, что есть свойство DaysOfHolidaysи функция AddHolidays(int).

Это потому, что личность сотрудника смешана с управлением праздниками, что плохо. Личность сотрудника - это сложная вещь, необходимая для отслеживания отпуска, зарплаты, сверхурочной работы, представления того, кто в какой команде, ссылки на отчеты об эффективности и т. Д. Сотрудник также может изменить имя, фамилию или и то и другое и остаться прежним. работник. Сотрудники могут даже иметь несколько написаний своего имени, таких как ASCII и Unicode. Люди могут иметь от 0 до n имен и / или фамилий. Они могут иметь разные имена в разных юрисдикциях. Отслеживание личности сотрудника является достаточной обязанностью, поэтому управление отпуском или отпуском нельзя добавить к началу, не называя это второй обязанностью.

«Я верю в SOLID. Вы нарушаете принцип единой ответственности (SRP), поскольку вы представляете данные и выполняете логику в одном классе».

Как и другие, я не согласен с этим.

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

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

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

Как это? Не следует стремиться к тому, чтобы выяснить, почему это может помочь снизить затраты на техническое обслуживание, и, надеюсь, это не должно быть предметом бесконечных дискуссий, но, как и в случае с SOLID, в общем, это не то, что можно применять вслепую повсюду. Это то, что нужно учитывать при балансе компромиссов.

Что касается вероятности требующих изменений, это уменьшается с:

1. Хорошее тестирование (улучшенная надежность).
2. Вовлечение только минимального кода, необходимого для выполнения чего-то конкретного (это может включать в себя уменьшение афферентных связей).
3. Просто сделайте код плохим в том, что он делает (см. Принцип Make Badass).

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

Принцип «задира»: классы, которые используются во многих местах, должны быть классными. Они должны быть надежными, эффективными, если это связано с их качеством и т. Д.

А Принцип «Сделай Злобу» связан с Принципом Минимальной Скорби, так как злые вещи найдут меньшую вероятность требующих изменений, чем вещи, которые сосут из того, что они делают.

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

С точки зрения SRP у класса, который почти ничего не делает, наверняка есть только одна (иногда нулевая) причина для изменения:

class Float
{
public:
    explicit Float(float val);
    float get() const;
    void set(float new_val);
};

Это практически не имеет причин для изменения! Это лучше, чем SRP. Это ЗРП!

За исключением того, что я хотел бы предположить, что это является вопиющим нарушением принципа «Сделай Злобу» Это также абсолютно бесполезно. То, что делает так мало, не может надеяться быть задирой. В нем слишком мало информации (TLI). И, естественно, когда у вас есть что-то, что является TLI, оно не может сделать что-то действительно значимое, даже с информацией, которую оно инкапсулирует, поэтому оно должно просочиться в внешний мир в надежде, что кто-то на самом деле сделает что-то значимое и злобное. И эта утечка в порядке для чего-то, что просто предназначено для агрегирования данных и ничего более, но этот порог является разницей, как я вижу, между «данными» и «объектами».

Конечно, то, что является TMI, также плохо. Мы могли бы поместить все наше программное обеспечение в один класс. Это может даже иметь только один runметод. И кто-то может даже возразить, что теперь у него есть одна очень широкая причина для изменения: «Этот класс нужно менять только в том случае, если программное обеспечение нуждается в улучшении». Я глупый, но, конечно, мы можем представить все проблемы с обслуживанием.

Таким образом, существует баланс между гранулярностью или грубостью объектов, которые вы проектируете. Я часто судю об этом по тому, сколько информации вы должны просочиться во внешний мир, и сколько функциональных возможностей он может выполнять. Я часто нахожу там принцип Make Badass, чтобы найти баланс, сочетая его с принципом минимальной скорби.

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

Многие системы, как правило, делают это, когда несколько слоев должны играть с одними и теми же данными (сервисный уровень, веб-уровень, клиентский уровень, агенты ...).

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

Например, это может быть плохой идеей, если процесс агента, который вычисляет статистику по всем вашим сотрудникам, может вызвать вычисления для оплаченных дней. И GUI со списком сотрудников, конечно, вообще не нуждается в новых вычислениях агрегированных идентификаторов, используемых в этом статистическом агенте (и технических данных, поставляемых с ним). Когда вы разделяете методы таким образом, вы обычно заканчиваете классом только с структурами данных.

Вы можете слишком легко сериализовать / десериализовать данные «объекта», или только некоторые из них, или в другой формат (json) ... не беспокоясь о какой-либо концепции / ответственности объекта. Это просто передача данных. Вы всегда можете выполнить сопоставление данных между двумя классами (Employee, EmployeeVO, EmployeeStatistic ...), но что на самом деле здесь означает Employee?

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

Если вам не нужно разделять области действия, вы можете более свободно размещать методы в классах обслуживания или в доменных классах, в зависимости от того, как вы видите свой объект. Я склонен рассматривать объект как «реальную» концепцию, это, естественно, помогает сохранить SRP. Таким образом, в вашем примере это более реалистично, чем босс сотрудника, добавляющий выходной день, предоставленный его PayDayAccount. Сотрудник нанимается сотрудником компании Works, может быть больным или просить совета, и у него есть учетная запись Payday (босс может получить его непосредственно от него или из реестра PayDayAccount ...) Но вы можете сделать совокупный ярлык здесь для простоты, если вы не хотите слишком много сложности для простого программного обеспечения.

Вы нарушаете принцип единой ответственности (SRP), поскольку представляете данные и выполняете логику в одном классе.

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

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