2011-09-07 10 views
5

メソッドコール、たとえばClassA.search(a、b、flag)が3つのコントローラで使用されているこのコードが発生しました。これはメソッドの簡略化されたバージョンです。これはメソッドを再利用/共有する良い方法ですか?

public List<Result> search(Object a, Object b, boolean flag) { 
    //do some code logic here, common to the 3 controllers 
    //at the middle there is: 
    if (flag) { 
     //code that affects 2 Controllers 
    } else { 
     //code affects only 1 
    } 
    //some more common code 
    //some more code with the flag if else 
} 

コードは再利用されるので、これは良い考えですか?または、コードを再利用できるが、メソッド呼び出し元(クライアント)コードのカスタマイズ(この3つのメソッドを3つの異なるメソッドに分割しても、共通コードのリファクタリングされたメソッドを宣言できるように)にこのフラグを導入しない方が良い方法はありますか?

答えて

6

まず、抽出物は、関数での行をコメント:

public void search(Object a, Object b, boolean flag) 
{ 
    commonToThree(); 
    if (flag) 
    { 
     affectTwoControllers(); 
    } 
    else 
    { 
     affectsOnlyOne(); 
    } 
    alsoCommon(); 
} 

は今、コードのにおいですflagブール引数、取り除く:

public void searchWithTrueFlag(Object a, Object b) { 
    commonToThree(); 
    affectTwoControllers(); 
    alsoCommon(); 
} 

public void searchWithFalseFlag(Object a, Object b) { 
    commonToThree(); 
    affectsOnlyOne(); 
    alsoCommon(); 
} 
+0

ローカル変数をフィールドに変換して機能させることができなければ、私はこれに同意します。 –

+0

なぜローカル変数が悪いのですか?パラメータを何度も渡す必要がある状態が非常に重要な場合は、コンストラクタの最終フィールドに初期化された状態でワンタイムオブジェクトを作成し、一度だけ使用します。ステロイドの機能;-)。 –

+0

それは本当ですか?フィールドを持つクラス(およびコンストラクタ?)を追加することは、1つのフラグを避けるための多くの作業です。 ;) –

3

いいですがそれほど優れていません。 1つのブール値は理にかなっていますが、それ以上の追加を開始すると正しい方向に進まないことになります。

それは常に可能ではないのですが、一般的に行うために、より良いコードを生成:

functionOne: 
    sharedCodeOne() 
    specificCode 
    sharedCodeTwo() 

functionTwo: 
    sharedCodeOne() 
    specificCode 
    sharedCodeTwo() 

いつものように、一般的な主張をするのは難しい:これは常に明らかに実用的/ことはできません。

+1

+1:これは、コードの3つのセクションがローカル変数を共有しない場合に有効です。(または少数しか共有しない) –

+1

@Peter:私は完全に同意します - 私は、ローカル変数の数と範囲を減らすことをあなたに促すのは、このアプローチの実際的な利点であると思います。 (一部の人はそれらを囲いのクラスのフィールドに変えて、物事を混乱させる傾向があるかもしれませんが) –

+1

@Arnout Engelen、私はこの作業をするためのフィールドを追加することは良い考えではないと私は認めます。 –

0

これは比較的簡単な方法です。代替案がありますが、より複雑になる可能性があります。 (訪問者を渡すか、コントローラーのメソッドを呼び出してそのコントローラーのために何をするのかなど)

このアプローチは、コードの3つのセクション間でローカル変数を共有する場合に最も適しています。

0

私は答えにしようとは異なるアプローチを取ると思います一般的な方法でこの質問:

主な目標はクリーンコードです。正確にはこれはもちろん、特定のケースに依存します。しかし、コピーと貼り付けのコードをいくつかの場所に置くことは悪いことです。変更する必要がある場合はいくつかの場所を変更する必要があるからです。同様に、複数のパーツが使用している共通部分を抽出しようとするのは悪いすべての状況でそれら。

あなたのコードを読んでいる人(または今から数週間後にこれをやる必要がある人)は、いつも起こっていることをできるだけ早く理解しなければならないと想像してください。したがって、いくつかの行をコピーしたり、共通の方法で抽出したりする方が良い場合は、特殊なケースに依存します。

フラグは本質的に悪いものではありませんが、実際にはJavaメソッドで過度に使用されるべきものではありません。しばしば、このような状況は、メソッドのオーバーロードと他のメソッドのオーバーロードによってうまく解決することができます。そのため、共通のケースは1つに、特殊な追加はもう1つで行われます。あるいは、いくつかのサブメソッドを持ち、いくつかの呼び出しでメソッドを構成することができますが、あまりにも多くのパラメーターを渡す必要がない場合にのみ意味があります。

1つのルール(完全に主観的ですが、多くのプロジェクトから学んだレッスンに基づいています)私はあなたに与えることができます:具体的な実装を好む一般的なアプローチ。コードが増え、賢明に見えないかもしれませんが、理解し、拡張し、デバッグするのがはるかに簡単です。

関連する問題