c# - ビジネス検証ロジックコードの臭い

Question

次のコードを検討してください。

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

つまり、OurPropertyinの値をOurBusinessObject変更したときに、値が有効でない場合は、デフォルト値に設定します。このパターンはコードの臭いとして私を襲いますが、ここにいる他の人（私の雇用主）は同意しません。あなたの考えは何ですか？

追加するために編集：なぜこれが大丈夫だと思われるのかについての説明を追加するように頼まれました。ビジネスオブジェクトのプロデューサーにデータを検証させるのではなく、ビジネスオブジェクトが自身のプロパティを検証し、検証が失敗した場合にクリーンなデフォルト値を設定できるという考え方でした。さらに、検証ルールが変更された場合、ビジネスオブジェクトがデータの検証とクリーニングを処理するため、ビジネスオブジェクトのプロデューサーはロジックを変更する必要がないと考えられていました。

score 17 · Accepted Answer

それは絶対に恐ろしいです。本番環境で問題をデバッグしようと頑張ってください。それがもたらす可能性がある唯一のことは、バグをカバーすることです。バグはどこか別の場所に表示され、どこから来ているのかまったくわかりません。

score 3 · Accepted Answer

私はあなたに同意しなければならないと思います。これにより、ロジックが予期せずデフォルトに戻るという問題が発生する可能性があり、デバッグが非常に困難になる可能性があります。

少なくとも、この動作はログに記録する必要がありますが、これは例外をスローする場合のようです。

score 3 · Accepted Answer

私には、これは実際の問題ではなく、症状のように見えます。実際に起こっていることは、のセッターがイベントOurPropertyで使用するための元の値を保持できないことです。OnOurPropertyChangedそうすれば、突然、進め方についてより良い選択をすることが容易になります。

さらに言えば、本当に必要なのは、割り当てが実際に行われる前OnOurPropertyChangingにセッターから発生するイベントです。このようにして、最初に割り当てを許可または拒否できます。そうしないと、オブジェクトが無効になる時間が少しあります。つまり、型がスレッドセーフではなく、同時実行性が懸念される場合は一貫性を期待できません。

score 2 · Accepted Answer

間違いなく疑わしい慣行。

無効な値がこのプロパティに割り当てられるのはどうしてですか？これは、呼び出し元のコードのどこかにバグがあることを示しているのではないでしょうか。その場合は、すぐに知りたいと思うでしょう。または、ユーザーが何かを間違って入力した場合は、すぐに通知する必要がありますか？

一般に、「迅速に失敗する」と、バグの追跡がはるかに簡単になります。舞台裏でデフォルトをサイレントに割り当てることは「魔法」に似ており、コードベースを維持しなければならない人に混乱を引き起こすだけです。

score 1 · Accepted Answer

「コードの臭い」という用語の嫌悪感はさておき、あなたは正しいかもしれません-それがどこから来ているかによっては、値を黙って変更することはおそらく良いことではありません。デフォルトに戻すのではなく、値が有効であることを確認することをお勧めします。

score 1 · Accepted Answer

プロパティを設定する前に、検証するためにリファクタリングすることを強くお勧めします。

次のような方法を常に使用できます。

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

何をするかは問題ではありませんが、現在の値を変更する前に検証することが重要です。新しい値が適切かどうかを判断する前に値を変更すると、長期的には問題が発生します。

score 0 · Accepted Answer

「においがする」かもしれないし、そうでないかもしれませんが、私はもっと「においがする」に傾いています。

OurPropertyをデフォルトに設定することには論理的な理由がありますか、それともコードで設定するのが簡単ですか？実際にはありそうもないことですが、これが予想される動作になるシナリオを考案することは可能ですが、ほとんどの場合、例外をスローしてどこかできれいに処理する必要があると思います。

値をデフォルトに設定すると、アプリケーションがどのように機能するかについての機能仕様の説明に近づいたり、遠ざかったりしますか？

score 0 · Accepted Answer

変更が行われた後、変更を検証していますか？ビジーネスプロパティを変更する前に、検証を行う必要があります。

あなたの質問に答える：そのコードスニペットで提示された解決策は本番環境で大きな問題を引き起こす可能性があります。無効な入力が原因でデフォルト値が表示されたのか、他の何かが値をデフォルトに設定したために表示されたのかわかりません

score 0 · Accepted Answer

私はGrzenioに同意し、ドメイン層（別名ビジネスオブジェクト）で検証エラーを処理する最良の方法は例外を生成することであると付け加えます。この例外は、UIレイヤーにまで伝播し、そこでユーザーが処理してインタラクティブに修正できる可能性があります。ただし、関連する機能やテクノロジによっては、これが常に実行可能であるとは限りません。その場合は、おそらくUIレイヤー（おそらくドメインレイヤーに加えて）で検証する必要があります。理想的とは言えませんが、実行可能な唯一の選択肢かもしれません。いずれにせよ、デフォルト値に設定することは恐ろしいことであり、診断がほぼ不可能になる微妙なバグにつながります。大規模に実行すると、すぐに保守不可能なシステムになります（特に、バックアップする単体テストがない場合）。

score 0 · Accepted Answer

コンテキストやビジネスルールを知らずに言うのは難しいです。ただし、一般的に言えば、入力時に検証する必要があり、おそらく永続化の前にもう一度検証する必要がありますが、プロパティに無効な値を含めることを許可していないため、実際に検証することはできません。

score 0 · Accepted Answer

無効な値を使用するように求められた場合、検証ロジックで例外が発生するはずです。コンシューマーがデフォルト値を使用したい場合は、特別な文書化された値または別の方法を使用して、明示的に要求する必要があります。

私が許容できると考えることができる唯一の例外は、重複を検出するための電子メールフィールドのように、大文字と小文字を正規化することです。

score 0 · Accepted Answer

さらに、一体なぜこれが部分的なのか？生成されたコードフレームワーク以外の部分クラスを使用することは、それ自体がコードの香りです。

score 0 · Accepted Answer

私は、PropertyChanging を実装し、ビジネスロジックが値を承認/拒否できるようにし、その後、無効な値に対して例外をスローすると言います。

このようにして、無効な値を持つことはありません。それと、ユーザーの情報を決して変更しないでください。ユーザーがデータベースにエントリを追加し、自分の記録のためにそれを追跡するとどうなるでしょうか? あなたのコードは値をデフォルトに再割り当てし、彼は間違った情報を追跡しています. できるだけ早くユーザーに通知することをお勧めします。

score 0 · Accepted Answer

これに対して私が持っている反論は次のとおりです。ビジネスオブジェクトのユーザー/プロデューサーが誤って無効な値を入力したとします。次に、このパターンはその事実を覆い隠し、デフォルトでデータをクリーンにします。しかし、これを処理する正しい方法は、エラーをスローし、ユーザー/プロデューサーに入力データを検証/消去させることです。

c# - ビジネス検証ロジックコードの臭い

14 に答える 14

Related

Reference