Business Validation Logic Код пахнет

Question

Рассмотрят следующий код:

partial class OurBusinessObject { 
    partial void OnOurPropertyChanged() { 
     if(ValidateOurProperty(this.OurProperty) == false) { 
      this.OurProperty = OurBusinessObject.Default.OurProperty; 
     } 
    } 
} 

То есть, когда значение OurProperty в OurBusinessObject изменяется, если значение не является допустимым, установите его значение по умолчанию. Этот шаблон поражает меня как запах кода, но другие здесь (у моего работодателя) не согласны. Что ты думаешь?

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

Answer 1

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

Answer 2

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

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

Answer 3

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

Answer 4

Может или не «пахнет», но я больше склоняюсь к «Да, это пахнет».

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

Устанавливает ли значение значение по умолчанию, вы приближаетесь или отключаете вас от функционального ? Спецификация описания того, как приложение должно работать?

Answer 5

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

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

Answer 6

Вы подтверждаете изменения после того, как это было сделано? Валидация должна быть выполнена до изменения свойства занятости.

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

Answer 7

Определенно спорная практика.

Каким образом было бы присвоено недопустимое значение этому свойству? Разве это не указывает на то, что где-то в вызывающем коде есть ошибка, и в таком случае вы, вероятно, захотите сразу это узнать? Или что пользователь вводит что-то неправильно, в каком случае им необходимо сообщить сразу?

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

Answer 8

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

Вы всегда можете иметь метод, который был больше похож:

T GetValidValueForProperty<T>(T suggestedValue, T currentValue); 

или даже:

T GetValidValueForProperty<T>(string propertyName, T suggestedValue, T currentValue); 

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

Это будет использоваться больше как:

 
T OurProperty 
{ 
    get 
    { 
     return this.propertyBackingField; 
    } 
    set 
    { 
     this.propertyBackingField = this.GetValidValueForProperty(value, this.propertyBackingField); 
    } 
} 

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

Answer 9

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

Answer 10

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

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

Answer 11

Кроме того, почему в мире это частичное? Использование частичных классов для чего-либо, кроме сгенерированной структуры кода, само по себе является кодексом, поскольку вы, вероятно, используете их, чтобы скрыть сложность, которую нужно разделить в любом случае!

Answer 12

Я согласен с Grzenio и добавлю, что лучший способ справиться с ошибкой проверки на уровне домена (aka business objects) - это генерировать исключение. Это исключение может распространяться вплоть до уровня пользовательского интерфейса, где он может обрабатываться и интерактивно исправляться с пользователем.Однако, в зависимости от возможностей и технологий, это может оказаться не всегда возможным, и в этом случае вы, вероятно, должны проверять уровень UI (возможно, в дополнение к доменному слою). Это меньше, чем идеально, но может быть вашим единственным жизнеспособным вариантом. В любом случае установка его по умолчанию - это ужасная вещь, которая приведет к тонким ошибкам, которые невозможно диагностировать. Если это будет сделано в широком масштабе, у вас будет незаметная система в кратчайшие сроки (особенно если у вас нет модульных тестов, которые поддерживают вас).

Answer 13

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

Answer 14

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

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