10

考虑以下代码:

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

即当in的值OurProperty发生OurBusinessObject变化时,如果该值无效,则设置为默认值。这种模式让我觉得代码有异味,但这里的其他人(在我的雇主那里)不同意。你怎么认为?

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

4

14 回答 14

17

这绝对是可怕的。祝你在生产环境中调试问题好运。它唯一能导致的就是掩盖错误,这些错误只会出现在其他地方,在那里它们来自哪里根本不明显。

于 2009-03-11T17:22:54.493 回答
3

我想我必须同意你的看法。这肯定会导致逻辑意外返回默认值的问题,这可能很难调试。

至少,应该记录这种行为,但这似乎更像是引发异常的情况。

于 2009-03-11T17:22:29.563 回答
3

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

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

于 2009-03-11T17:28:46.273 回答
2

绝对是一个有问题的做法。

如何将无效值分配给此属性?这是否表明调用代码中的某处存在错误,在这种情况下您可能想立即知道?或者用户输入错误,在这种情况下应该立即通知他们?

一般来说,“快速失败”可以更容易地跟踪错误。在幕后默默地分配默认值类似于“魔术”,只会给必须维护代码库的人造成混乱。

于 2009-03-11T17:38:40.180 回答
1

除了厌恶“代码气味”这个词,你可能是对的——取决于它的来源,默默地改变值可能不是一件好事。最好确保您的值有效,而不是仅仅恢复为默认值。

于 2009-03-11T17:23:19.780 回答
1

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

你总是可以有一个更像的方法:

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);
    }
}

您做什么并不重要,但重要的是在更改当前值之前进行验证。如果您在确定新值是否合适之前更改了值,那么从长远来看,您是在自找麻烦。

于 2009-03-11T18:01:08.503 回答
0

它可能会或可能不会“闻起来”,但我更倾向于“是的,它闻起来”。

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

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

于 2009-03-11T17:27:18.647 回答
0

您正在验证完成后的更改吗?验证应该在busyness 属性改变之前完成。

回答您的问题:该代码段中提供的解决方案可能会在生产中产生大问题,您不知道默认值是由于输入无效还是因为其他东西将值设置为默认值而出现在那里

于 2009-03-11T17:38:22.257 回答
0

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

于 2009-03-11T18:05:43.003 回答
0

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

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

于 2009-03-11T18:08:26.270 回答
0

此外,这世界上为什么是局部的?除了生成的代码框架之外,对任何东西使用部分类本身就是一种代码味道,因为您可能会使用它们来隐藏复杂性,无论如何都应该将其拆分!

于 2009-03-11T18:12:31.613 回答
0

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

于 2009-03-11T18:25:42.970 回答
0

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

于 2009-03-11T18:40:21.750 回答
0

我会说,实现 PropertyChanging 并允许业务逻辑批准/拒绝一个值,然后为无效值抛出异常。

这样,您永远不会有无效的值。那,你不应该改变用户的信息。如果用户向数据库添加一个条目,并为自己的记录跟踪它会怎样?您的代码会将值重新分配给默认值,而他现在正在跟踪错误的信息。最好尽快通知用户。

于 2009-03-17T12:43:51.163 回答