Законная «настоящая работа» в конструкторе?

23

Я работаю над дизайном, но продолжаю преодолевать препятствия. У меня есть определенный класс (ModelDef), который по сути является владельцем сложного дерева узлов, созданного путем анализа XML-схемы (например, DOM). Я хочу следовать хорошим принципам проектирования (SOLID) и гарантировать, что полученная система легко тестируется. У меня есть все намерения использовать DI для передачи зависимостей в конструктор ModelDef (чтобы их можно было легко заменить при необходимости во время тестирования).

Однако я борюсь с созданием дерева узлов. Это дерево будет полностью состоять из простых «ценностных» объектов, которые не нужно будет проверять независимо. (Тем не менее, я все еще могу передать Абстрактную Фабрику в ModelDef, чтобы помочь в создании этих объектов.)

Но я продолжаю читать, что конструктор не должен выполнять какую-либо реальную работу (например, Flaw: Constructor делает реальную работу ). Это имеет смысл для меня, если «настоящая работа» означает создание тяжелых зависимых объектов, которые позже можно было бы заглушить для тестирования. (Они должны быть переданы через DI.)

Но как насчет легких объектов значений, таких как это дерево узлов? Дерево должно быть где-то создано, верно? Почему не через конструктор ModelDef (используя, скажем, метод buildNodeTree ())?

Я действительно не хочу создавать дерево узлов вне ModelDef, а затем передавать его (через конструктор DI), потому что создание дерева узлов путем синтаксического анализа схемы требует значительного количества сложного кода - кода, который необходимо тщательно протестировать , Я не хочу отрицать это, чтобы «склеить» код (который должен быть относительно тривиальным и, скорее всего, не будет напрямую проверен).

Я думал о том, чтобы поместить код для создания дерева узлов в отдельный объект «строитель», но стесняюсь называть его «строитель», потому что он на самом деле не соответствует шаблону построителя (который, как представляется, больше касается устранения телескопирования). конструкторы). Но даже если бы я назвал это как-то по-другому (например, NodeTreeConstructor), он все равно выглядит как хак, просто чтобы избежать конструктора ModelDef для построения дерева узлов. Это должно быть где-то построено; почему не в объекте, который будет владеть им?

Gurtz
источник
7
Вы всегда должны быть осторожны с такими заявлениями. Общее практическое правило - это код таким образом, чтобы он был понятным, функциональным, простым в тестировании, повторном использовании и обслуживании, каким бы способом он ни был, который зависит от вашей ситуации. Если вы получаете только сложность кода и путаницу, пытаясь следовать такому «правилу», тогда это не будет подходящим правилом для вашей ситуации. Все эти «шаблоны» и языковые функции являются инструментами; используйте лучший для вашей конкретной работы.
Джейсон С

Ответы:

26

И, помимо того, что предложил Росс Паттерсон, рассмотрим эту позицию, которая является полной противоположностью:

  1. Возьмите принципы, такие как «Не делай никакой реальной работы в конструкторах своих» с крошкой соли.

  2. На самом деле конструктор - это не что иное, как статический метод. Таким образом, структурно разница между:

    а) простой конструктор и набор сложных статических фабричных методов, и

    б) простой конструктор и несколько более сложных конструкторов.

Значительная часть негативных настроений по отношению к выполнению какой-либо реальной работы в конструкторах происходит из определенного периода истории C ++, когда велись споры относительно того, в каком именно состоянии останется объект, если в конструкторе выдается исключение, и деструктор должен быть вызван в таком случае. Эта часть истории C ++ закончилась, и проблема была решена, в то время как в таких языках, как Java, никогда не было подобных проблем с самого начала.

Мое мнение таково: если вы просто избегаете использования newв конструкторе (как показывает ваше намерение использовать Dependency Injection), все будет в порядке. Я смеюсь над такими утверждениями, как «условная или циклическая логика в конструкторе является предупреждающим знаком недостатка».

Помимо всего этого, лично я бы убрал логику синтаксического анализа XML из конструктора не потому, что плохо иметь сложную логику в конструкторе, а потому, что хорошо следовать принципу «разделения интересов». Итак, я бы переместил логику синтаксического анализа XML в отдельный класс, а не в статические методы, принадлежащие вашему ModelDefклассу.

поправка

Я полагаю, что если у вас есть метод, вне ModelDefкоторого создается объект ModelDefиз XML, вам нужно будет создать некоторую динамическую временную древовидную структуру данных, заполнить ее синтаксическим анализом XML, а затем создать новую ModelDefпередачу этой структуры в качестве параметра конструктора. Таким образом, это можно рассматривать как применение шаблона «Строитель». Существует очень тесная аналогия между тем, что вы хотите сделать , и String& StringBuilderпару. Тем не менее, я нашел этот вопрос и ответ, который, кажется, не согласен по причинам, которые мне не ясны: Stackoverflow - StringBuilder и Builder Pattern . Таким образом, чтобы избежать длительной дискуссии о том, StringBuilderреализует или нет модель «строителя», я бы сказал, не стесняйтесь вдохновляться тем, какStrungBuilder работает над созданием решения, которое соответствует вашим потребностям, и откладывает называние его приложением шаблона «Построитель» до тех пор, пока эта маленькая деталь не будет решена.

Посмотрите на этот новый вопрос: Программисты SE: Является ли «StringBuilder» приложением шаблона проектирования Builder?

Майк Накис
источник
3
@RichardLevasseur Я просто помню, что это было предметом беспокойства и споров среди программистов на C ++ в начале и середине девяностых. Если вы посмотрите на этот пост: gotw.ca/gotw/066.htm, вы увидите, что это довольно сложно, а довольно сложные вещи, как правило, вызывают споры. Я не знаю наверняка, но я думаю, что в начале девяностых часть этого материала еще даже не была стандартизирована. Но извините, я не могу предоставить хорошую ссылку.
Майк Накис
1
@Gurtz Я бы подумал о таком классе, как о специфичных для приложения «утилитах xml», поскольку формат файла xml (или структура документа), вероятно, связан с конкретным приложением, которое вы разрабатываете, независимо от каких-либо возможностей повторно использовать ваш «ModelDef».
Майк Накис
1
@Gurtz, я бы, наверное, сделал их экземплярами методов основного класса «Application», или, если это слишком хлопотно, то статическими методами некоторого вспомогательного класса, способом, очень похожим на то, что предложил Росс Паттерсон.
Майк Накис
1
@Gurtz приносит свои извинения за то, что не обращался к подходу «строитель» ранее. Я исправил свой ответ.
Майк Накис
3
@Gurtz Это возможно, но вне академического любопытства это не имеет значения. Не втягивайтесь в «шаблонный анти-шаблон». Шаблоны на самом деле являются просто именами, чтобы удобно описывать общие / полезные методы кодирования другим. Делайте то, что вам нужно, добавьте ярлык позже, если вам нужно это описать. Вполне нормально реализовать что-то, что «похоже на шаблон конструктора, в некотором смысле, может быть», если ваш код имеет смысл. При изучении новых методов разумно сосредоточиться на шаблонах, просто не думайте, что все, что вы делаете, должно быть каким-то именованным шаблоном.
Джейсон C
9

Вы уже даете веские основания не делать эту работу в ModelDef конструкторе:

  1. Нет ничего «легкого» в разборе XML-документа на дерево узлов.
  2. Там нет ничего очевидного о ModelDef том, что он может быть создан только из XML-документа.

Похоже, ваш класс должен иметь множество статических методов, таких как ModelDef.FromXmlString(string xmlDocument), ModelDef.FromXmlDoc(XmlDoc parsedNodeTree)и т. Д.

Росс Паттерсон
источник
Спасибо за ответ! По поводу предложения статических методов. Будут ли это статические фабрики, создающие экземпляр ModelDef (из различных источников XML)? Или они будут нести ответственность за загрузку уже созданного объекта ModelDef? Если последнее, я бы беспокоился о том, чтобы объект был инициализирован только частично (поскольку ModelDef нужно, чтобы дерево узлов было полностью инициализировано). Мысли?
Гурц
3
Извините, что согласился, но да, Росс имеет в виду статические фабричные методы, которые возвращают полностью построенные экземпляры. Полный прототип будет что-то вроде public static ModelDef createFromXmlString( string xmlDocument ). Это довольно распространенная практика. Иногда я тоже так делаю. Мое предположение о том, что вы также можете делать только конструкторы, является моим стандартным типом ответа в ситуациях, когда я подозреваю, что альтернативные подходы отвергаются как «не кошерные» без уважительной причины.
Майк Накис
1
@ Майк-Накис, спасибо за разъяснения. Таким образом, в этом случае метод статической фабрики должен построить дерево узлов, а затем передать его в конструктор (возможно частный) из ModelDef. Имеет смысл. Спасибо.
Гурц
@Gurtz Точно.
Росс Паттерсон
5

Я слышал это «правило» раньше. По моему опыту это и правда, и ложь.

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

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

Таким образом, мы получаем другую «школу мысли», которая является ТВЕРДЫЙ. И в этой школе мысли, скорее всего, гораздо более верно, что мы не должны выполнять настоящую работу в конструкторе. Код SOLID часто (но не всегда) легче тестировать. Но также может быть сложнее рассуждать. Мы разбиваем наш код на маленькие объекты с единственной ответственностью, и, таким образом, большинство наших объектов больше не инкапсулируют свое состояние (и обычно содержат либо состояние, либо поведение). Код валидации обычно извлекается в класс валидатора и хранится отдельно от состояния. Но теперь мы потеряли сплоченность, мы больше не можем быть уверены, что наши объекты действительны, когда мы их получим и будем полностьюконечно, мы должны всегда проверять, что предварительные условия, которые мы считаем имеющимися в отношении объекта, верны, прежде чем мы попытаемся что-то сделать с объектом. (Конечно, обычно вы делаете проверку в одном слое, а затем предполагаете, что объект действителен в нижних слоях.) Но это проще для тестирования!

Так кто же прав?

Никто на самом деле. Обе школы мысли имеют свои достоинства. В настоящее время SOLID в моде, и все говорят о SRP, Open / Closed и всем этом джазе. Но то, что что-то популярно, не означает, что это правильный выбор дизайна для каждого отдельного приложения. Так что это зависит. Если вы работаете в кодовой базе, которая строго следует принципам SOLID, то да, реальная работа в конструкторе, вероятно, плохая идея. Но в противном случае, посмотрите на ситуацию и попытайтесь использовать свое суждение. Какие свойства ваш объект получает от выполнения работы в конструкторе, какие свойства он теряет ? Насколько это соответствует общей архитектуре вашего приложения?

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

wasatz
источник
Это фантастический ответ.
Джрахали
0

Существует фундаментальная проблема с этим правилом, и именно это составляет «настоящую работу»?

Вы можете увидеть из оригинальной статьи опубликованной в вопросе, что автор пытается определить, что такое «настоящая работа», но она имеет серьезные недостатки. Чтобы практика была хорошей, она должна быть четко определенным принципом. Под этим я подразумеваю, что в отношении разработки программного обеспечения идея должна быть переносимой (независимой от любого языка), проверенной и доказанной. Большая часть того, что обсуждается в этой статье, не соответствует этим первым критериям. Вот некоторые показатели, которые автор упоминает в этой статье о том, что представляет собой «настоящая работа» и почему они не являются плохими определениями.

Использование newключевого слова . Это определение в корне неверно, потому что оно специфично для предметной области. Некоторые языки не используют newключевое слово. В конечном итоге он намекает на то, что он не должен конструировать другие объекты. Однако во многих языках даже самые основные ценности сами являются объектами. Таким образом, любое значение, назначенное в конструкторе, также создает новый объект. Это делает эту идею ограниченной определенными языками и плохим показателем того, что составляет «реальную работу».

Объект не полностью инициализирован после завершения конструктора . Это хорошее правило, но оно также противоречит нескольким другим правилам, упомянутым в этой статье. Хорошим примером того, как это может противоречить другим, является упомянутый вопрос, который привел меня сюда. В этом вопросе кто-то обеспокоен использованиемsort метода в конструкторе, который выглядит как JavaScript из-за этого принципа. В этом примере человек создавал объект, представляющий отсортированный список других объектов. В целях обсуждения представьте, что у нас есть несортированный список объектов, и нам нужен новый объект для представления отсортированного списка. Нам нужен этот новый объект, потому что некоторая часть нашего программного обеспечения ожидает отсортированный список и позволяет вызывать этот объектSortedList, Этот новый объект принимает несортированный список, и результирующий объект должен представлять теперь отсортированный список объектов. Если бы мы следовали другим правилам, упомянутым в этом документе, а именно: никаких статических вызовов методов, никаких структур потока управления, ничего, кроме присваивания, то результирующий объект не был бы создан в допустимом состоянии, нарушая другое правило его полной инициализации. после завершения конструктора. Чтобы это исправить, нам нужно выполнить некоторую базовую работу, чтобы отсортировать несортированный список в конструкторе. Это нарушит 3 других правила, делая другие правила неактуальными.

В конечном итоге это правило не выполнять «настоящую работу» в конструкторе плохо определено и ошибочно. Попытка определить, что такое «настоящая работа», является бесполезным упражнением. Лучшее правило в этой статье - когда конструктор заканчивает работу, он должен быть полностью инициализирован. Существует множество других лучших практик, которые могут ограничить объем работы, выполняемой в конструкторе. Большинство из них могут быть обобщены в принципах SOLID, и эти же принципы не помешают вам выполнять работу в конструкторе.

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

zquintana
источник