Образец Строителя: Когда терпеть неудачу?

45

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

Сначала несколько объяснений:

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

Тогда несколько аргументов:

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

Каково общее мнение об этом?

skiwi
источник
8
Я не вижу никакого преимущества в том, чтобы терпеть неудачу поздно. То, что кто-то говорит, что класс строителя «должен» быть, не имеет приоритета над хорошим дизайном, и ранний поиск ошибок всегда лучше, чем поздний.
Довал
3
Другой способ взглянуть на это состоит в том, что сборщик может не знать, что является действительными данными. В этом случае неудача на раннем этапе больше связана с неудачей, как только вы узнаете, что произошла ошибка. Не потерпев неудачу рано, конструктор вернет nullобъект, когда возникла проблема в build().
Крис
Если вы не добавите способ выдачи предупреждений и предложите средства для исправления в сборщике, нет смысла опаздывать поздно.
Марк

Ответы:

34

Давайте посмотрим на варианты, где мы можем разместить код проверки:

  1. Внутри сеттеры у застройщика.
  2. Внутри build()метода.
  3. Внутри build()созданного объекта: он будет вызываться в методе при создании объекта.

Вариант 1 позволяет нам обнаруживать проблемы раньше, но могут быть сложные случаи, когда мы можем проверять ввод только с полным контекстом, таким образом, делая по крайней мере часть проверки в build()методе. Таким образом, выбор варианта 1 приведет к несогласованности кода, при котором часть проверки выполняется в одном месте, а другая часть - в другом.

Вариант 2 не намного хуже, чем вариант 1, потому что, как правило, сеттеры в компоновщике вызываются непосредственно перед build(), особенно в свободно используемых интерфейсах. Таким образом, в большинстве случаев все еще возможно обнаружить проблему достаточно рано. Однако, если построитель - не единственный способ создания объекта, это приведет к дублированию проверочного кода, поскольку он должен быть везде, где вы создаете объект. Наиболее логичным решением в этом случае будет поставить проверку как можно ближе к созданному объекту, то есть внутри него. И это вариант 3 .

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

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

UPD: этот комментарий напомнил мне еще одну возможность, когда проверка внутри компоновщика (вариант 1 или 2) имеет смысл. Это имеет смысл, если у застройщика есть свои собственные контракты на объекты, которые он создает. Например, предположим, что у нас есть конструктор, который создает строку с определенным содержимым, скажем, список диапазонов номеров 1-2,3-4,5-6. Этот строитель может иметь метод, как addRange(int min, int max). Результирующая строка ничего не знает об этих числах, и не должна знать. Сам строитель определяет формат строки и ограничения на числа. Таким образом, метод addRange(int,int)должен проверять введенные числа и генерировать исключение, если max меньше min.

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

Иван Гаммель
источник
Я думаю, что стоит отметить, что, хотя вариант 1 может привести к «непоследовательному» времени проверки, он все равно может рассматриваться как непротиворечивый, если все «как можно раньше». Немного проще сделать «как можно раньше» более определенным, если вместо этого используется вариант построителя, StepBuilder.
Джошуа Тейлор
Если конструктор URI выдает исключение, если передается пустая строка, является ли это нарушением SOLID? Мусор
Гусдор
@ Гусдор да, если он сам выдает исключение. Однако, с точки зрения пользователя, все параметры выглядят так, как будто их создает исключение.
Иван Гаммель
Так почему бы не иметь validate (), который вызывается build ()? Таким образом, существует небольшое дублирование, согласованность и отсутствие нарушений SRP. Это также позволяет проверять данные, не пытаясь построить, и проверка близка к созданию.
StellarVortex
@StellarVortex в этом случае будет проверен дважды - один раз в builder.build (), и, если данные верны, и мы переходим к конструктору объекта, в этом конструкторе.
Иван Гаммель
34

Учитывая, что вы используете Java, рассмотрите авторитетное и подробное руководство, предоставленное Джошуа Блохом в статье « Создание и уничтожение объектов Java» (жирный шрифт в приведенной ниже цитате - мой):

Как конструктор, конструктор может наложить инварианты на свои параметры. Метод сборки может проверить эти инварианты. Очень важно, чтобы они были проверены после копирования параметров из компоновщика в объект и чтобы они были проверены в полях объекта, а не в полях компоновщика (элемент 39). Если какие-либо инварианты нарушены, метод сборки должен выдать IllegalStateException(Item 60). Метод детализации исключения должен указывать, какой инвариант нарушен (элемент 63).

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

Обратите внимание, что согласно пояснениям редактора этой статьи, «элементы» в приведенной выше цитате относятся к правилам, представленным в Effective Java, Second Edition .

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

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

Подумайте, что спортивный автомобиль не может иметь более двух мест, как можно узнать, setSeats(4)хорошо это или нет? Это только при сборке, когда можно точно знать, setSportsCar()был ли вызван или нет, то есть, бросить TooManySeatsExceptionили нет.

комар
источник
3
+1 за рекомендацию, какие типы исключений выбрасывать, именно то, что я искал.
Xantix
Не уверен, что я получу альтернативу. Кажется, речь идет исключительно о том, когда инварианты могут быть проверены только в группах. Построитель принимает отдельные атрибуты, когда они не связаны с какими-либо другими, и принимает группы атрибутов, только когда группа имеет инвариант. В этом случае должен ли единственный атрибут генерировать исключение перед сборкой?
Дидье А.
19

Неверные значения, которые являются недопустимыми, потому что они недопустимы, должны быть немедленно объявлены, по моему мнению. Другими словами, если вы принимаете только положительные числа и передается отрицательное число, нет необходимости ждать вызова build(). Я бы не стал рассматривать эти типы проблем, которые вы «ожидаете», поскольку это является обязательным условием для вызова метода для начала. Другими словами, вы вряд ли будете зависеть от сбоя установки определенных параметров. Скорее всего, вы предполагаете, что параметры верны, или проведете некоторую проверку самостоятельно.

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

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

Тем не менее, это лучшая практика, чем все остальное. Я надеюсь, что это отвечает на ваш вопрос.

Нил
источник
11

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

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

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

  • Требовать, чтобы оба (или более) атрибута были предоставлены одновременно (то есть вызов одного метода).
  • Проверьте правильность, как только вы узнаете, что больше никаких изменений не поступило: когда build()или около того вызывается.

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

JVR
источник
Итак, подводя итог, вы говорите, что разумно как можно раньше проверить все, что могло бы быть охвачено в объектном / примитивном типе? Как unsigned, @NonNullи т.д.
skiwi
2
@skiwi Довольно много, да. Доменные проверки, нулевые проверки, такие вещи. Я бы не стал вкладывать в это гораздо больше: строители - это, как правило, простые вещи.
JvR
1
Может быть, стоит отметить, что если достоверность одного параметра зависит от значения другого, можно только законно отклонить значение параметра, если один знает, что другой «действительно» установлен . Если допустимо устанавливать значение параметра несколько раз [с последним значением, имеющим приоритет], то в некоторых случаях наиболее естественным способом установки объекта может быть установка параметра Xна значение, которое недопустимо, учитывая текущее значение Y, но перед вызовом build()установите Yзначение, которое сделает Xдействительным.
суперкат
Если, например, кто-то строит, Shapeа у строителя есть WithLeftи WithRightсвойства, и кто-то хочет настроить построителя так, чтобы он строил объект в другом месте, требуя, чтобы WithRightон вызывался первым при перемещении объекта вправо и WithLeftпри перемещении его влево, добавило бы ненужную сложность по сравнению с разрешением WithLeftустановить левый край справа от старого правого края при условии, что WithRightправый край фиксируется до buildвызова.
суперкат
0

Основное правило - «рано провалиться».

Чуть более продвинутое правило - «провалиться как можно раньше».

Если свойство изначально недействительно ...

CarBuilder.numberOfWheels( -1 ). ...  

... тогда вы немедленно отвергаете это.

В других случаях может потребоваться проверка значений в комбинации, и их лучше разместить в методе build ():

CarBuilder.numberOfWheels( 0 ).type( 'Hovercraft' ). ...  
Фил В.
источник