2012-02-25 10 views
0

私は登録プロセスの一歩を表すクラスを持っています。私はステップを記入した後にユーザーが保存をクリックした後、私たちがステップを実行して、登録プロセスの終了時にステップを実行したいと思っている他のものを、ステップその段階。私は州があるというアイデアを使用することに決めましたが、悪いコードの臭いがあるようです。このデザインを改善する方法に関するコメント?コードと他の問題を無視オブジェクト指向設計の問題

public class Step1 
{ 
    public Enum State 
    { 
     InProcess = 1, 
     EndProcess 
    } 

    private State processState; 

    public Step1(State currentState) 
    { 
     processState = currentState; 
    } 

    public bool IsValid() 
    { 
     bool result; 

     if(processState = State.InProcess) 
     { 
      result = PerformCheck1(); 
     } 
     else if(processState = State.EndProcess) 
     { 
      result = PerformCheck2(); 
      result = PerformCheck3(); 
     } 
     else 
     { 
      throw new Exception("Cannot determine process state"); 
     } 

     return result; 
    } 

    public void Save() 
    { 
     if(processState = State.InProcess) 
     { 
      DoThing1(); 
     } 
     else if(processState = State.EndProcess) 
     { 
      DoThing2(); 
      DoThing3(); 
      DoThing4(); 
     } 
     else 
     { 
      throw new Exception("Cannot determine process state"); 
     } 
    } 
} 
+2

はcodereviewに属します。 –

答えて

1

あなたは、この設計で行く場合は、ステップのために、単一のモノリシッククラスに巻くしようとしています。私はISTEPインターフェイスを作成し、それぞれが独自のクラスをステップになるだろう:

public interface IStep 
{ 
    bool IsValid { get; } 
    void Save(); 
} 

public class BeginStep : IStep 
{ 
    public bool IsValid 
    { 
     get 
     { 
      return PerformCheck1(); 
     } 
    } 

    public void Save() 
    { 
     DoThing1(); 
    } 
} 

public class EndStep : IStep 
{ 
    public bool IsValid 
    { 
     get 
     { 
      // Skipped PerformCheck2() since the result is directly overwritten 
      return PerformCheck3(); 
     } 
    } 

    public void Save() 
    { 
     DoThing2(); 
     DoThing3(); 
     DoThing4(); 
    } 
} 
+0

各ステップは自分のクラスになる予定でした。サンプル・クラスはStep1と呼ばれ、ステップを保存すると特定の保存アクションがあるのに対して、ステップ5(最終ステップ)にあるときは以前のすべてのステップでsaveが呼び出されますが、最終段階のプロセスです。 – user1054637

0

私はあなたの答え(+ジャスティンの答えにあなたのコメントを)undersdand場合は正しく、各ステップは、「処理」にすることができ、「完了します」どの状態にあるかによって、異なるSave()IsValid()のメソッドを呼び出す必要があります。

最後に、すべてのステップが自動的に「完了」されます。

投稿したコードから、彼が現在どの状態にいるかを知る必要があるかのようには見えません。最終ページにいるかどうかによって、異なるアクションを実行するだけです。

"完了した"ロジックをカプセル化する特定のメソッド(例:SaveCompleted()およびIsValidCompleted())を作成したくない理由はありますか?

ステップが実行するコードを決定する理由はわかりません。なぜなら、ステップが実行中のステップを追跡しているステップの外側で決定されるからです。

外部から変更できる隠し状態に基づいて自動的に適切な処理を行う「マジックメソッド」は、しばしば不整合、予期しない問題、長いデバッグセッションにつながります。

ジャスティンが提案したように、「完了」コードを含む2つの追加メソッドを使用してインターフェイスを作成することをお勧めします。 (「完了した」メソッドがデフォルトで「処理」メソッドになる可能性がある)または別のインターフェイス(「完了済み」メソッドのみを公開するという利点があります)最終的なページに、それはregualrメソッドを呼び出すことを可能にせずに)など)

+0

私はちょうど、個々のメソッドに分離するのではなく、論理フローを見てメソッドを見せてくださっていると思ったと思います。私はあなたの意見を取ります。あなたの投稿の最後にも提案を見ていきます – user1054637

0

ジャスティンの答えに加えて、ここでは、作成しようとしているものがFinite-state machineへのウィキペディアのリンクです有限状態機械。このリンクは、テーマに関する理論的背景知識を提供します。登録プロセスがより複雑になる場合は、この記事で説明したState/Eventテーブルがイベントの流れを定式化するのに役立ちます。

関連する問題