2009-03-11 11 views
10

は、次のコードを考えてみましょ香り:ビジネスの検証ロジックコードが

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

値が有効でない場合OurBusinessObjectOurPropertyの値は、変更されたとき、ある、デフォルト値に設定します。このパターンはコードの匂いとして私を襲うが、ここ(私の雇用主の)他の人は同意しない。あなたの考えは?

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

答えて

17

本当に恐ろしいです。プロダクションの問題をデバッグしようとすると幸いです。それがもたらすことができる唯一のものはバグをカバーすることです。バグはどこかに現れ、どこから来ているのかはっきりしません。

+0

ほとんどの場合、私は同意しますが、悪いデータが時々クリープすることが予想される場合、一部のプロパティはデフォルトで安全にデフォルトできます。状況はどういうものかわからない。 –

+0

プロパティが安全にデフォルト設定されていれば、なぜ最初にそれらのプロパティを要求するのですか?あなたはとにかくユーザーが提供しているものを無視している! –

3

私はあなたに同意する必要があると思います。これは間違いなくロジックが予期せずデフォルトに戻り、デバッグが非常に難しい問題につながる可能性があります。

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

1

「コードの匂い」という言葉の嫌悪感は別として、あなたが正しいと思うかもしれません。どこから来るのかによって、値を静かに変更することはおそらく良いことではありません。デフォルトに戻すのではなく、値が有効であることを確認する方がよいでしょう。

0

"匂い"があるかもしれないし、そうでないかもしれないが、私は "はい、それは匂い"に向かって傾いている。

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

機能のアプリケーションの動作の仕組みの説明に近づけたり、離れたりすることはありますか?

3

私にとってこれは実際の問題ではなく症状のようです。実際に何が起こっているのは、OurPropertyの設定者がイベントで使用するために元の値を保持できないということです。そうするならば、突然、進め方についてより良い選択をする方が簡単になります。そのことについては

割り当てが実際に行われるする前に、あなたが本当にしたいことはセッターから上げてOnOurPropertyChangingイベントです。この方法では、最初に割り当てを許可または拒否できます。さもなければ、あなたのオブジェクトが有効でない少量の時間があります。これは、型がスレッドセーフではないことを意味します。並行性が懸念される場合は、一貫性を考慮することはできません。

+2

流通は楽しいです。 –

0

変更が完了した後で変更を検証していますか?バリデーションは、ビジーネスプロパティが変更される前に行う必要があります。

あなたquestingに答える:そのコードスニペットで提示ソリューションは、生産に大きな問題を発生させることができ、デフォルト値は無効な入力またはちょうど何か他のものがデフォルト

に値を設定するので、そこに現れたかどうかわかりません
2

間違いなく疑わしい練習です。

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

一般に、「失敗する」と表示されると、バグの追跡が非常に簡単になります。舞台裏でデフォルトを静かに割り当てることは "魔法"に似ており、コードベースを維持しなければならない人には混乱を招くだけです。

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

これは、より多くのように使用されます。新しい価値が良いかどうかを判断する前に価値を変更した場合は、長期的に問題を求めています。

0

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

0

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

私が考えることができる唯一の例外は、重複を検出する電子メールフィールドのように、正規化の場合と同様です。

0

さらに、なぜこの部分的なのですか?作成されたコードフレームワーク以外の部分クラスを使用することは、それ自体がコードメッセージであるため、複雑さを隠すために使用する可能性が高いためです。

+0

部分的に本質的に悪いと私は同意しません。 cf. 1つの使用のためのhttp://msdn.microsoft.com/en-us/library/bb738612.aspx – jason

+0

このオブジェクトはEF(タグ)を使用していると言われているので、おそらくEFデザイナーから作成されています。この場合、部分クラスが最適なオプションです。 –

0

私はGrzenioに同意し、ドメインレイヤー(別名ビジネスオブジェクト)で検証エラーを処理する最良の方法は、例外を生成することだと付け加えます。その例外は、UIレイヤーにすべて伝播して処理され、ユーザーと対話的に修正される可能性があります。しかし、関連する機能や技術によっては、これが必ずしも実現可能なわけではないかもしれません。その場合は、おそらくUIレイヤー(おそらくドメインレイヤー)で検証する必要があります。それは理想的ではありませんが、唯一の実行可能な選択肢かもしれません。いずれにせよ、デフォルト値に設定するのは恐ろしいことであり、微妙なバグが発生し、診断が不可能に近くなります。広範囲に行われた場合、時間の無駄なシステムを維持することができます(特に単体テストをバックアップしていない場合)。

0

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

0

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

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