业务验证逻辑代码异味

Question

考虑以下代码：

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

即当 OurBusinessObject 中的 OurProperty 的值发生更改时，如果该值无效，则将其设置为默认值。 这种模式给我的印象是代码味道，但这里的其他人（我的雇主）不同意。 你怎么看？

编辑添加：我被要求添加一个解释，说明为什么认为这是可以的。 这个想法是，业务对象可以验证其自己的属性，并在验证失败时设置干净的默认值，而不是让业务对象的生产者验证数据。 此外，人们认为，如果验证规则发生变化，业务对象生产者不必更改其逻辑，因为业务对象将负责验证和清理数据。

Answer 1

这绝对是可怕的。 祝您尝试调试生产中的问题好运。 它唯一可能导致的就是掩盖错误，这些错误只会在其他地方突然出现，而它们的来源根本不明显。

Answer 2

我想我必须同意你的观点。 这肯定会导致逻辑意外返回默认值的问题，这可能非常难以调试。

至少，应该记录这种行为，但这看起来更像是抛出异常的情况。

Answer 3

对我来说，这看起来像是症状，而不是实际的问题。 真正发生的事情是 OurProperty 的 setter 无法保留在 OnOurPropertyChanged 事件中使用的原始值。 如果你这样做了，突然之间就可以更容易地做出更好的选择来继续进行。

就此而言，您真正想要的是在分配实际发生之前从 setter 引发的 OnOurPropertyChanging 事件。 这样您就可以首先允许或拒绝该分配。 否则，在一小段时间内，您的对象无效，这意味着该类型不是线程安全的，如果您认为并发是一个问题，则不能指望一致性。

Answer 4

绝对是一种值得怀疑的做法。

如何为该属性分配无效值？ 这是否表明调用代码中存在错误，在这种情况下您可能想立即知道？ 或者用户输入了错误的内容，在这种情况下应该立即通知他们？

一般来说，“快速失败”使得追踪错误变得更加容易。 在幕后默默地分配默认值类似于“魔术”，只会给维护代码库的人带来混乱。

Answer 5

撇开对“代码味道”这个术语的厌恶不谈，你可能是对的 - 取决于它来自哪里，默默地改变值可能不是一件好事。 最好确保您的值有效，而不是仅仅恢复为默认值。

Answer 6

我强烈建议在设置属性之前重构它以进行验证。

您总是可以有一个更像是的方法：

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 7

它可能有也可能没有“气味”，但我更倾向于“是的，它闻起来”。

将 OurProperty 设置为默认值是否有逻辑上的原因，或者在代码中这样做只是为了方便？ 尽管在实践中不太可能，但有可能设计出一种预期行为的场景，但我猜测在大多数情况下，您应该抛出异常并在某个地方干净地处理它。

将值设置为默认值是否会让您更接近或远离应用程序应如何工作的功能规范描述？

Answer 8

您要在更改完成后验证更改吗？ 应在更改繁忙度属性之前进行验证。

回答您的问题：该代码片段中提供的解决方案可能会在生产中产生大问题，您不知道默认值是否由于无效输入而出现在那里，或者只是因为其他原因将该值设置为默认值

Answer 9

在不了解上下文或业务规则的情况下很难说。 一般来说，您应该在输入时进行验证，也许在持久化之前再次进行验证，但是您这样做的方式实际上并不允许您进行验证，因为您不允许属性包含无效值。

Answer 10

我认为如果要求使用无效值，您的验证逻辑应该引发异常。 如果您的消费者想要使用默认值，则应该通过特殊的记录值或通过其他方法明确要求。

我认为唯一可以原谅的例外是，例如，规范化大小写，例如在电子邮件字段中检测重复项。

Answer 11

而且，到底为什么会有这样的偏心呢？ 对除生成的代码框架之外的任何内容使用部分类本身就是一种代码味道，因为您可能使用它们来隐藏无论如何都应该拆分的复杂性！

Answer 12

我同意 Grzenio 的观点，并补充说，处理域层（也称为业务对象）中的验证错误的最佳方法是生成异常。 该异常可能会一直传播到 UI 层，在那里可以与用户交互地处理和纠正异常。 但是，根据所涉及的功能和技术，这可能并不总是可行，在这种情况下，您可能应该在 UI 层（可能除了域层之外）进行验证。 它不太理想，但可能是您唯一可行的选择。 无论如何，将其设置为默认值都是一件可怕的事情，并且会导致几乎无法诊断的细微错误。 如果大规模完成，您很快就会拥有一个无法维护的系统（特别是如果您没有单元测试支持）。

Answer 13

我反对这一点的一个论点如下。 假设业务对象的用户/生产者意外地输入了无效值。 然后，该模式将掩盖该事实并默认为干净的数据。 但处理这个问题的正确方法是抛出错误并让用户/生产者验证/清理他们的输入数据。

Answer 14

我想说，实现 PropertyChanging 并允许业务逻辑批准/拒绝某个值，然后针对无效值抛出异常。

这样，您就不会得到无效值。 也就是说，您永远不应该更改用户的信息。 如果用户向数据库添加一个条目并跟踪它作为自己的记录怎么办？ 您的代码会将值重新分配给默认值，而他现在正在跟踪错误的信息。 最好尽快通知用户。