Я стараюсь максимально придерживаться принципа единой ответственности (SRP) и привык к определенной схеме (для SRP по методам), в значительной степени полагаясь на делегатов. Я хотел бы знать, является ли этот подход обоснованным или есть ли какие-либо серьезные проблемы с ним.
Например, чтобы проверить ввод в конструктор, я мог бы ввести следующий метод ( Stream
ввод случайный, может быть любым)
private void CheckInput(Stream stream)
{
if(stream == null)
{
throw new ArgumentNullException();
}
if(!stream.CanWrite)
{
throw new ArgumentException();
}
}
Этот метод (возможно) делает больше чем одно
- Проверьте входы
- Бросай разные исключения
Чтобы придерживаться SRP, я изменил логику
private void CheckInput(Stream stream,
params (Predicate<Stream> predicate, Action action)[] inputCheckers)
{
foreach(var inputChecker in inputCheckers)
{
if(inputChecker.predicate(stream))
{
inputChecker.action();
}
}
}
Который предположительно делает только одну вещь (делает это?): Проверьте вход. Для фактической проверки входных данных и выдачи исключений я ввел такие методы, как
bool StreamIsNull(Stream s)
{
return s == null;
}
bool StreamIsReadonly(Stream s)
{
return !s.CanWrite;
}
void Throw<TException>() where TException : Exception, new()
{
throw new TException();
}
и может звонить CheckInput
как
CheckInput(stream,
(this.StreamIsNull, this.Throw<ArgumentNullException>),
(this.StreamIsReadonly, this.Throw<ArgumentException>))
Это вообще лучше, чем первый вариант, или я ввел ненужную сложность? Можно ли как-то улучшить этот паттерн, если он вообще жизнеспособен?
источник
CheckInput
все еще делает несколько вещей: это итерация по массиву и вызов функции предиката и вызов функции действия. Разве это не является нарушением ПСП?Ответы:
SRP, пожалуй, самый неправильно понятый программный принцип.
Программное приложение построено из модулей, которые построены из модулей, которые построены из ...
Внизу, одна функция, такая как,
CheckInput
будет содержать лишь небольшую часть логики, но по мере продвижения вверх каждый последующий модуль инкапсулирует все больше и больше логики, и это нормально .SRP - это не одно атомное действие. Речь идет о единоличной ответственности, даже если эта обязанность требует нескольких действий ... и, в конечном итоге, об обслуживании и тестировании :
Тот факт, что
CheckInput
он реализован с двумя проверками и вызывает два разных исключения, в некоторой степени не имеет значения .CheckInput
несет узкую ответственность: обеспечение соответствия входных данных требованиям. Да, существует несколько требований, но это не означает, что существует множество обязанностей. Да, вы можете разделить чеки, но как это поможет? В какой-то момент проверки должны быть перечислены каким-то образом.Давайте сравним:
против:
Теперь
CheckInput
делает меньше ... но звонящий делает больше!Вы переместили список требований от того
CheckInput
, где они заключены, к тому,Constructor
где они видны.Это хорошая перемена? По-разному:
CheckInput
вызывался только там: это спорно, с одной стороны , это делает требование видно, с другой стороны , это загромождает код;CheckInput
вызывается несколько раз с одними и теми же требованиями , это нарушает DRY, и у вас возникает проблема инкапсуляции.Важно понимать, что одна ответственность может означать много работы. «Мозг» самостоятельного вождения автомобиля несет единственную ответственность:
Это единственная ответственность, но она требует координации тонны сенсоров и действующих лиц, принятия большого количества решений и даже, возможно, противоречивых требований 1 ...
... однако, все это заключено в капсулу. Так что клиенту все равно.
1 безопасность пассажиров, безопасность других, соблюдение правил, ...
источник
Цитата дяди Боба о SRP ( https://8thlight.com/blog/uncle-bob/2014/05/08/SingleReponsibilityPrinciple.html ):
Он объясняет, что программные модули должны решать конкретные проблемы заинтересованных сторон. Поэтому, отвечая на ваш вопрос:
ИМО, вы смотрите только на один метод, когда вы должны смотреть на более высокий уровень (уровень класса в данном случае). Возможно, нам следует взглянуть на то, что в настоящее время делает ваш класс (и это требует более подробного объяснения вашего сценария). Пока ваш класс все еще делает то же самое. Например, если завтра будет какой-то запрос на изменение о какой-либо проверке (например, «теперь поток может быть нулевым»), то вам все равно нужно перейти к этому классу и изменить его содержимое.
источник
checkInputs()
их следует разделить, скажем, наcheckMarketingInputs()
иcheckRegulatoryInputs()
. В противном случае можно объединить их все в один метод.Нет, это изменение не сообщается SRP.
Спросите себя, почему в вашем контролере нет проверки на то, что «переданный объект является потоком» . Ответ очевиден: язык не позволяет вызывающей программе компилировать программу, которая передается в потоке.
Система типов C # недостаточна для удовлетворения ваших потребностей; Ваши проверки реализуют принудительное выполнение инвариантов, которые сегодня не могут быть выражены в системе типов . Если бы был способ сказать, что метод принимает поток, доступный для записи, не допускающий установки в нуль, вы бы написали это, но это не так, поэтому вы сделали следующую лучшую вещь: вы установили ограничение типа во время выполнения. Надеюсь, вы также задокументировали это, так что разработчикам, которые используют ваш метод, не нужно нарушать его, проваливать свои тестовые примеры, а затем исправлять проблему.
Размещение типов в методе не является нарушением принципа единой ответственности; ни метод не обеспечивает выполнение своих предварительных условий или утверждение его предварительных условий.
источник
Не все обязанности созданы равными.
Вот два ящика. У них обоих есть одна ответственность. У каждого из них есть имена, которые сообщают вам, что в них принадлежит. Одним из них является ящик для серебра. Другой ящик для мусора.
Так в чем же разница? Ящик для столового серебра ясно показывает, что в нем нет. Однако ящик для мусора принимает все, что подойдет. Извлекать ложки из ящика для столового серебра кажется очень неправильным. Тем не менее, мне трудно думать о чем-либо, что было бы пропущено, если бы его убрали из ящика для мусора. Правда в том, что вы можете утверждать, что на что-либо лежит одна ответственность, но как вы думаете, на чем лежит более сфокусированная ответственность?
Объект с единственной ответственностью не означает, что здесь может произойти только одна вещь. Обязанности могут гнездиться. Но эти обязанности должны иметь смысл, они не должны удивлять вас, когда вы найдете их здесь, и вы должны пропустить их, если они ушли.
Поэтому, когда вы предлагаете
CheckInput(Stream stream);
Я не обеспокоен тем, что он одновременно проверяет ввод и генерирует исключения. Я был бы обеспокоен, если бы это одновременно проверяло и сохраняло ввод. Это неприятный сюрприз. Я бы не пропустил, если бы его не было
источник
Когда вы связываете себя узлами и пишете странный код, чтобы соответствовать Принципу важного программного обеспечения, обычно вы неправильно понимаете этот принцип (хотя иногда этот принцип неверен). Как указывает превосходный ответ Матье, весь смысл СРП зависит от определения «ответственности».
Опытные программисты видят эти принципы и связывают их с воспоминаниями о коде, который мы облажались; менее опытные программисты видят их и могут вообще не иметь к ним никакого отношения. Это абстракция, плавающая в космосе, вся улыбка и нет кота. Так они и догадываются, и обычно это плохо Прежде чем вы начнете понимать смысл программирования, разница между странным, чрезмерно сложным кодом и обычным кодом совсем не очевидна.
Это не религиозная заповедь, которой вы должны подчиняться независимо от личных последствий. Это скорее практическое правило, призванное формализовать один элемент программирования и помочь вам сделать ваш код максимально простым и понятным. Если это имеет противоположный эффект, вы правы, чтобы искать какой-то внешний вклад.
В программировании, вы не можете пойти так заблуждаться , чем пытаться вывести значение идентификатора из первых принципов, просто глядя на него, и это касается идентификаторов в письменной форме о программировании так же , как идентификаторы в настоящем коде.
источник
CheckInput роль
Во-первых, позвольте мне изложить очевидное
CheckInput
: делать одну вещь, даже если она проверяет различные аспекты. В конечном итоге он проверяет ввод . Можно утверждать, что это не одно, если вы имеете дело с вызываемыми методамиDoSomething
, но я думаю, что можно с уверенностью предположить, что проверка ввода не слишком расплывчата.Добавление этого шаблона для предикатов может быть полезно, если вы не хотите, чтобы логика проверки входных данных помещалась в ваш класс, но этот шаблон кажется довольно многословным для того, чего вы пытаетесь достичь. Возможно, гораздо проще просто передать интерфейс
IStreamValidator
одним методом,isValid(Stream)
если вы этого хотите. Любой реализующий классIStreamValidator
может использовать предикаты, какStreamIsNull
или,StreamIsReadonly
если они того пожелают, но, возвращаясь к центральной точке, это довольно нелепое изменение в интересах поддержания принципа единой ответственности.Санитарная проверка
Я считаю, что нам всем разрешена «проверка работоспособности», чтобы гарантировать, что вы, по крайней мере, имеете дело с потоком, который не является пустым и доступен для записи, и эта базовая проверка не делает ваш класс валидатором потоков. Имейте в виду, что более сложные проверки лучше оставить за пределами вашего класса, но именно здесь проходит черта. Как только вам нужно начать изменять состояние своего потока, читая его или выделяя ресурсы для проверки, вы начали выполнять формальную проверку вашего потока, и это то, что следует включить в его собственный класс.
Заключение
Я думаю, что если вы применяете шаблон, чтобы лучше организовать аспект вашего класса, он заслуживает того, чтобы быть в своем классе. Поскольку шаблон не подходит, вы также должны спросить, действительно ли он действительно принадлежит своему классу. Я думаю, что если вы не верите, что валидация потока, вероятно, изменится в будущем, и особенно если вы верите, что эта валидация может даже иметь динамический характер, описанный вами шаблон является хорошей идеей, даже если он может быть изначально тривиальным. В противном случае, нет необходимости произвольно делать вашу программу более сложной. Давайте называть вещи своими именами. Проверка - это одно, но проверка на нулевой ввод не является проверкой, и поэтому я думаю, что вы можете безопасно хранить ее в своем классе, не нарушая принцип единственной ответственности.
источник
Принцип категорически не гласит, что кусок кода должен «делать только одну вещь».
«Ответственность» в ПСП должна пониматься на уровне требований. Ответственность кода заключается в удовлетворении бизнес-требований. SRP нарушается, если объект удовлетворяет нескольким независимым бизнес-требованиям . Независимость означает, что одно требование может измениться, а другое требование остается в силе.
Вполне возможно, что введено новое бизнес-требование, которое означает, что этот конкретный объект не должен проверять на предмет читабельности, в то время как другое бизнес-требование все еще требует, чтобы объект проверял на читабельность? Нет, потому что бизнес-требования не определяют детали реализации на этом уровне.
Фактическим примером нарушения SRP был бы код, подобный этому:
Этот код очень прост, но все же возможно, что текст будет меняться независимо от ожидаемой даты поставки, поскольку это решается различными частями бизнеса.
источник
Мне нравится пункт из ответа @ EricLippert :
Эрик Липперт прав, что это проблема системы типов. А поскольку вы хотите использовать принцип единой ответственности (SRP), вам, в основном, нужна система типов, чтобы отвечать за эту работу.
На самом деле это возможно сделать в C #. Мы можем ловить литералы
null
во время компиляции, а затем ловить не-литералыnull
во время выполнения. Это не так хорошо, как полная проверка во время компиляции, но это строгое улучшение по сравнению с тем, чтобы никогда не ловить во время компиляции.Итак, вы знаете, как C # имеет
Nullable<T>
? Давайте изменим это и сделаемNonNullable<T>
:Теперь вместо того, чтобы писать
, просто пиши:
Тогда есть три варианта использования:
Пользователь звонит
Foo()
с ненулевым значениемStream
:Это желаемый вариант использования, и он работает с или без
NonNullable<>
.Пользователь звонит
Foo()
с нулемStream
:Это ошибка вызова. Здесь
NonNullable<>
помогает сообщить пользователю, что они не должны делать это, но это на самом деле не останавливает их. В любом случае, это приводит к времени выполненияNullArgumentException
.Пользователь звонит
Foo()
сnull
:null
не будет неявно преобразовываться в aNonNullable<>
, поэтому пользователь получит ошибку в IDE перед выполнением. Это делегирование нулевой проверки системе типов, как советует SRP.Вы можете расширить этот метод, чтобы утверждать и другие вещи о своих аргументах. Например, так как вы хотите записываемый поток, вы можете определить,
struct WriteableStream<T> where T:Stream
который проверяет какnull
иstream.CanWrite
в конструкторе. Это все равно будет проверка типа во время выполнения, но:Он украшает тип с помощью
WriteableStream
классификатора, сигнализируя о необходимости вызывающим.Он выполняет проверку в одном месте кода, поэтому вам не нужно повторять проверку и
throw InvalidArgumentException
каждый раз.Он лучше соответствует SRP, выдвигая обязанности по проверке типов в систему типов (как это было расширено общими декораторами).
источник
Ваш подход в настоящее время процедурный. Вы разбиваете
Stream
объект на части и проверяете его снаружи. Не делай этого - это нарушает инкапсуляцию. ПозвольтеStream
быть ответственным за свою собственную проверку. Мы не можем пытаться применить SRP, пока у нас не появятся классы для его применения.Вот то,
Stream
которое выполняет действие, только если оно проходит проверку:Но теперь мы нарушаем SRP! «У класса должна быть только одна причина измениться». У нас есть смесь 1) проверки и 2) реальной логики. У нас есть две причины, по которым это может измениться.
Мы можем решить это с помощью проверяющих декораторов . Во-первых, нам нужно преобразовать наш
Stream
интерфейс и реализовать его как конкретный класс.Теперь мы можем написать декоратор, который
Stream
упаковывает, выполняет валидацию и отсылает к заданномуStream
для реальной логики действия.Теперь мы можем составить их так, как нам нравится:
Хотите дополнительную проверку? Добавьте еще один декоратор.
источник
Работа класса заключается в предоставлении услуги, соответствующей договору . У класса всегда есть контракт: набор требований для его использования и обещания, которые он предъявляет о своем состоянии и результатах при условии, что требования выполнены. Этот контракт может быть явным, посредством документации и / или утверждений, или неявным, но он всегда существует.
Часть контракта вашего класса заключается в том, что вызывающая сторона предоставляет конструктору некоторые аргументы, которые не должны быть нулевыми. Реализация контракта является обязанностью класса, поэтому проверка того, что вызывающая сторона выполнила свою часть контракта, может легко рассматриваться как входящая в сферу ответственности класса.
Идея о том, что класс реализует контракт, принадлежит Бертрану Мейеру , дизайнеру языка программирования Eiffel и идеи дизайна по контракту . Язык Eiffel делает спецификацию и проверку контрактной части языка.
источник
Как указывалось в других ответах, SRP часто неправильно понимают. Дело не в атомарном коде, который выполняет только одну функцию. Речь идет о том, чтобы ваши объекты и методы выполняли только одно, а одно - только в одном месте.
Давайте посмотрим на плохой пример в псевдокоде.
В нашем довольно абсурдном примере «обязанность» конструктора Math # состоит в том, чтобы сделать математический объект пригодным для использования. Это делается сначала путем дезинфекции ввода, а затем путем проверки того, что значения не равны -1.
Это действительный SRP, потому что конструктор делает только одно. Он готовит объект Math. Однако это не очень ремонтопригодно. Это нарушает СУХОЙ.
Итак, давайте сделаем еще один проход на это
В этом проходе мы немного улучшили DRY, но у нас все еще есть способы использовать DRY. SRP с другой стороны, кажется, немного не в порядке. Теперь у нас есть две функции с одной и той же работой. И cleanX, и cleanY дезинфицируют ввод.
Давайте попробуем еще раз
Теперь, наконец, стало лучше насчет DRY, и SRP, похоже, согласен У нас есть только одно место, которое выполняет «санитарную» работу.
Код теоретически более удобен в обслуживании и лучше, но когда мы собираемся исправить ошибку и исправить код, нам нужно сделать это только в одном месте.
В большинстве реальных случаев объекты будут более сложными, и SRP будет применяться к группе объектов. Например, возраст может принадлежать отцу, матери, сыну, дочери, поэтому вместо четырех классов, определяющих возраст с даты рождения, у вас есть класс Person, который делает это, и 4 класса наследуют его. Но я надеюсь, что этот пример поможет объяснить. SRP не об атомарных действиях, а о выполнении «работы».
источник
Говоря о SRP, дяде Бобу не нравятся нулевые проверки, разбросанные повсюду. Как правило, вы, как команда, должны по возможности избегать использования нулевых параметров для конструкторов. Когда вы публикуете свой код за пределами вашей команды, все может измениться.
Обеспечение необнуляемости параметров конструктора без предварительного обеспечения связности рассматриваемого класса приводит к раздутию в вызывающем коде, особенно в тестах.
Если вы действительно хотите обеспечить соблюдение таких контрактов, рассмотрите возможность использования
Debug.Assert
или чего-то подобного, чтобы уменьшить беспорядок:источник