2011-10-20 10 views
9

私は2つの重要な機能を持つ1つのクラスを持っている:クラスでの呼び出しの順序が重要な場合のベストプラクティス

public class Foo { 
    //plenty of properties here 
    void DoSomeThing(){/*code to calculate results*/} 
    void SaveSomething(){/* code to save the results in DB*/} 

} 

SaveSomething()DoSomeThing()で計算した結果を使用しています。

DoSomeThing()の前にSaveSomething()に電話してはいけないという問題があります。そうした場合、結果は真の結果ではありません。私は呼び出しの順序が重要であることを意味します。これはコードを維持する際の問題です(新しいものがチームに追加されたとき)。

これを管理する方法はありますか?

bool resultsAreCalculated = false; 
void SaveSomething(){ 
    if (!resultsAreCalculated) { 
     DoSomeThing(); 
     // the resultsAreCalculated = true; is set in DoSomeThing(); 
     // can we throw some exception? 
    } 
    /* code to save the results in DB*/ 
} 

は、私はそれがDoSomeThing()SaveSomething()コードの変更に設定されているboolを持つDoSomeThing()

  • 前に呼び出された場合

    1. SaveSomething()に例外をスロー以下のように3つの方法を考えます
    2. それを実現するような流暢:

      この場合、
      Foo x = new Foo(); 
      x.DoSomeThing().SaveSomething(); 
      

      、起こるされていないことを保証することが重要です。

      x.SaveSomething().DoSomeThing(); 
      

    を今、私は第二の方法を使用します。何か良い方法があるのか​​、それとも十分ですか?

  • +2

    それは完全ではないので、これは、実際にコードレビューはありません。これは設計上の問題なので、Stack Overflowに投稿したいと思うかもしれません。 –

    +0

    @ S.Lottはい、そうですが、ここでは完全なコードではないと思います。私は完全なコードを新しい質問として投稿し、ここで2つまたは3つの答えを述べると、より多くのコードレビューを感じる。ところで、この質問は、([リンク]に関連するhttp://codereview.stackexchange.com/questions/5201/best-design-pattern-for-refactoring-a-set-of-classes-that-do-calculation-base -に)。最初の打ち上げから1.5年後にすべてのチームメンバーが新しい(このプロジェクトではプログラミングではなく)! –

    +0

    コードレビューは完全なコードです。すべての設計の決定後。これは「レビュー」が意味することです。つまり、すべての作業が作成された後に作成されることを意味します。デザインの問題はStack Overflowになります。 –

    答えて

    6

    ユーザーエラーを回避するための1つのオプションは、変数を渡すことで明確にすることです。これにより、SaveSomething(...)を呼び出す前に結果を取得する必要がある(DoSomething())というフラグが立てられます。

    results = DoSomething(); // returns the results to be saved 
    SaveSomething(results); 
    
    +0

    この答えではちょっと混乱しています.2番目の部分だけをアップヴォートする方法がないからです。 +1とにかく – Snowbear

    +0

    編集:最初の部分が削除されました。あなたは正しい - それは直接質問に答えるよりも接している。いいですよ。 – bitsoflogic

    +0

    答えに2番目の部分を追加するのは悪くないです。 –

    13

    理想的には、実行時に特定の順序に従う必要があるメソッドは、ある種のワークフローを示すか、実装する必要があることを暗示します。

    Template Method PatternまたはStrategyのような、ワークフローのような線形実行順序を実施するのをサポートするデザインパターンがいくつかあります。

    Template Methodアプローチを取るために、あなたのFooクラスはDo()Save()の実行順序を定義する抽象基盤を持つことになります、のようなもの:

    public abstract class FooBase 
    { 
        protected abstract void DoSomeThing(); 
        protected abstract void SaveSomething(); 
        public void DoAndSave() 
        { 
         //Enforce Execution order 
         DoSomeThing(); 
         SaveSomething(); 
        } 
    
    } 
    
    public class Foo : FooBase 
    { 
        protected override void DoSomeThing() 
        { 
         /*code to calculate results*/ 
        } 
    
        protected override void SaveSomething() 
        { 
         /* code to save the results in DB*/ 
        } 
    } 
    

    あなたのクラスの消費者は唯一DoAndSave()にアクセスする必要があります。この道であり、彼らはあなたが意図した実行の順序を変えることはありません。

    状況のワークフロー/状態遷移タイプを扱う別のパターンがあります。 Chain of CommandおよびStateパターンを参照することができます。あなたのコメントを受けて

    : これは、同じテンプレートの考え方を次の、あなたが保存する前に結果を確認したい想像し、あなたがなるために、あなたのテンプレートを拡張することができ、あなたのテンプレートに別のステップを追加するには:

    public abstract class FooBase 
    { 
        protected abstract void DoSomeThing(); 
        protected abstract void SaveSomething(); 
        protected abstract bool AreValidResults(); 
        public void DoAndSave() 
        { 
         //Enforce Execution order 
         DoSomeThing(); 
    
         if (AreValidResults()) 
          SaveSomething(); 
        } 
    
    } 
    

    もちろん、もっと精巧なワークフローのために、元の回答の最後に状態パターンを参照したので、ある状態から別の状態への遷移条件をより細かく制御することができます。

    +1

    途中結果が別の部分で必要な場合はどうすればよいですか?例えば ​​'DoSomeThing()'の結果がユーザに表示され、次に同意すれば(どこかをクリック)、 'SaveSomething()'が起こるでしょうか? –

    +1

    私の編集した答えを見てください..コメントセクションは長い返事に対応できませんでした。 –

    +0

    これは、DoAndSave()メソッドのメンテナーにとってはあまりにも漠然としていることに注意してください。この場合、@ Anas Karkoukliのコメントの重要性に注目してください。コメントは、コードがメンテナーによって壊されないようにするものになります。 – bitsoflogic

    2

    DoSave方法は私には順序対のように思えるではありません。 Doメソッドから計算の状態を返さないためにのみ注文する必要があります。結果をクライアントコードに返すメソッドとしてDoメソッドを記述すると、Saveを書き換えて結果をパラメータとして受け取ることができます。

    利点:

    1. Save方法は、クライアントがパラメータを得た方法気にしないので、あなたは、もうメソッドを注文する必要はありません。それだけでそれを受け取ります。
    2. メソッドがより結合しにくくなるため、ユニットテストDoメソッドを簡単に行うことができます。
    3. 複雑な保存ロジックを記述するか、リポジトリパターンを実装する必要がある場合は、Saveメソッドを別のクラスに移動することができます。 (私は担当者を持っていた場合は+1)Levinaris'と答えた時に拡大
    2

    は、あなたは、代わりにオブジェクトがDoSomthing()メソッドから返される結果にSave()メソッドを持つことができます。ですから、このようなものになるだろう。明らかに、このアプローチが返さresultsオブジェクトが結果を処分/保存を取り扱うジェネリック型のどちらかですが、また、一般的な結果が含まれていることが必要となる

    var obj = new Foo(); 
    
    // Get results 
    var results = obj.DoSomething(); 
    
    // Check validity, and user acceptance 
    if(this.AreValidResults(results) && this.UserAcceptsResults(results)) 
    { 
        // Save the results 
        results.Save(); 
    } 
    else 
    { 
        // Ditch the results 
        results.Dispose(); 
    } 
    

    を。特定の結果型が継承できる何らかの基本クラスの形式である必要があります。

    2

    私はAnas Karkoukliの答えが好きですが、もう1つの選択肢は状態マシンです。

    public class Foo { 
    
        private enum State { 
         AwaitingDo, 
         AwaitingValidate, 
         AwaitingSave, 
         Saved 
        } 
    
        private State mState = State.AwaitingDo; 
    
        private void Do() { 
    
         // Do something 
         mState = State.AwaitingValidate; 
        } 
    
        private void Validate() { 
    
         // Do something 
         mState = State.AwaitingSave; 
        } 
    
        private void Save() { 
    
         // Do something 
         mState = State.Saved; 
        } 
    
        public void MoveToNextState() { 
         switch (mState) { 
          case State.AwaitingDo: 
           Do(); 
           break; 
    
          case State.AwaitingValidation: 
           Validate(); 
           break; 
    
          case State.AwaitingSave: 
           Save(); 
           break; 
    
          case State.Saved: 
           throw new Exception("Nothing more to do."); 
           break; 
         } 
        } 
    } 
    

    これは少しスラッシュダッシュですが、あなたはそのアイデアを得ます。

    Anasの回答の問題は、すべての機能が1つのステップとして実行されることです。つまり、オブジェクトの中間段階には到達できません。ステートマシンは開発者にワークフローに従うよう強制しますが、ワークフローの各ステージでは、オブジェクトのプロパティを調べてから次のオブジェクトに移動することができます。

    2

    スティーブ・マクネルの優れた本コード・コンプリートは、この質問について議論しています。第2版​​の第14章です。

    ステートメントの順序が重要な場合は、その順序をデータで強制することをお勧めします。だから、

    calculateResults(); 
    saveResults(); 
    

    (インスタンス変数に結果を保存するには)

    Results r = calculateResults(); 
    saveResults(r); 
    

    を書くのではなく、彼らが計算される前に結果を保存しようとし、その後は非常に困難です。予想される注文がどれかを明確に示しています。これについて

    +0

    はい、そして 'Results'はある日、例えば' UpdateUI(r); 'のように使えます。 –

    3

    どのように?

    interface Result { 
        void Save(); 
        SomeData GetData(); 
    } 
    class Foo { 
        Result DoSomething() { /* ... */ } 
    } 
    

    使用法:

    myFoo.DoSomething().Save(); 
    //or something like: 
    var result = myFoo.DoSomething(); 
    if (result.GetData().Importance > threshold) result.Save(); 
    

    外部の視点から、これは多くの意味になります。 Resultが生成され、必要に応じて保存の手段が提供されますが、実装は完全に不透明です。私はこれを右のFooインスタンスに戻すことについて心配する必要はありません。実際には、結果をオブジェクトに渡すことができます。オブジェクトを作成したFooのインスタンスはわかりません(作成者は作成時に結果に保存するために必要なすべての情報を渡す必要があります)。結果は、それが必要であれば、既に保存されているかどうか、私に教える方法を持つかもしれません。等々。

    これは、基本的には主にインタフェースではなく実装上のもののSRPのちょうどアプリケーションです。 Fooのインターフェイスは結果を生成する手段を提供し、Resultは結果を操作する手段を意味します。

    関連する問題