2010-11-24 11 views
8

私はフラグ引数を持つメソッドを持っています。私はブール値をメソッドに渡すことは悪い習慣であると考えています(署名を複雑にし、 "各メソッドは一つのことをします"という原則に違反します)。私は、このメソッドを2つの異なるメソッドに分割する方が良いと思います。しかし、私がそれをするならば、2つの方法は非常によく似ています(コードの重複)。フラグ引数を持つメソッドを分割する方法はありますか?

フラグ引数を持つメソッドを2つの別々のメソッドに分割する一般的な技術があるのだろうかと思います。ここで

は私の方法(Javaの)のコードです:

int calculateNumOfLiveOrDeadNeighbors(Cell c, int gen, boolean countLiveOnes) { 
    int x = c.getX(); 
    int y = c.getY(); 
    CellState state; 
    int aliveCounter = 0; 
    int deadCounter = 0; 
    for (int i = x - 1; i <= x + 1; i++) { 
     for (int j = y - 1; j <= y + 1; j++) { 
     if (i == x && j == y) 
      continue; 
     state = getCell(i, j).getCellState(gen); 
     if (state == CellState.LIVE || state == CellState.SICK){ 
      aliveCounter++; 
     } 
     if(state == CellState.DEAD || state == CellState.DEAD4GOOD){ 
      deadCounter++; 
     } 
     } 
    } 
    if(countLiveOnes){ 
     return aliveCounter; 
    } 
    return deadCounter; 
} 

答えて

3

を私はそれがすべての単一のケースに依存推測します。

この例では、私の意見では2つの選択肢があります。

は、次の2つに呼び出し calculateNumOfLiveOrDeadNeighbors()

を分割したいと言う:

calculateNumOfLiveNeighbors() 

calculateNumOfDeadNeighbors() 

あなたは他の方法にループを移動するためにTemplate Methodを使用することができます。 これを使用して、2つの方法でデッド/アライブセルをカウントできます。

private int countCells(Cell c, int gen, Filter filter) 
{ 
    int x = c.getX(); 
    int y = c.getY(); 
    CellState state; 
    int counter = 0; 
    for (int i = x - 1; i <= x + 1; i++) 
    { 
     for (int j = y - 1; j <= y + 1; j++) 
     { 
      if (i == x && j == y) 
       continue; 
      state = getCell(i, j).getCellState(gen); 
      if (filter.countMeIn(state)) 
      { 
       counter++; 
      } 
     } 
    } 
    return counter; 
} 

private interface Filter 
{ 
     boolean countMeIn(State state); 
} 

public int calculateNumOfDeadNeighbors(Cell c, int gen) 
{ 
    return countCells(c, gen, new Filter() 
         { 
          public boolean countMeIn(CellState state) 
          { 
           return (state == CellState.DEAD || state == CellState.DEAD4GOOD); 
          } 
         }); 
    } 

public int calculateNumOfLiveNeighbors(Cell c, int gen) 
{ 
    return countCells(c, gen, new Filter() 
         { 
          public boolean countMeIn(CellState state) 
          { 
           return (state == CellState.LIVE || state == CellState.SICK); 
          } 
         }); 
    } 

痛いほど価値がないかもしれません。代わりにmonadを使用して統計計算の結果を保存し、モナドのgetDeadCounter()またはgetLiveCounter()を使用することができます。

+0

良いアイデアですが、静的内部クラスを使用してフィルタを実装します。 – Ralph

4
  • あなたは、単一の方法で共通の機能を抽出しようとだけあなたはプライベートメソッドを作成することができ、特定の機能
  • を使用することができますそのフラグで2つのパブリックメソッドから呼び出すことができます。したがって、公開APIには「複雑な」メソッドシグネチャはなく、重複コードはありません。
  • 両方の値を返すメソッドを作成し、各呼び出し元(パブリックメソッド)で1つを選択します。

上記の例では、2番目と3番目のオプションがより適用可能だと思います。

1

最も意味論的にきれいなアプローチのように見えるのは、両方の値を含む結果オブジェクトを返すことであり、呼び出しコードが結果オブジェクトから気にするものを抽出できるようにします。あなたがあなたの署名の真偽値が気に入らない場合

+0

+1最適化のため。さもなければ、メソッドは死んで生きている隣人を得るために二回呼び出されるべきです。 – khachik

5

、あなたはprivateにメインのそれなしで二つの異なる方法、リファクタリングを追加することができます。

int calculateNumOfLiveNeighbors(Cell c, int gen) { 
    return calculateNumOfLiveOrDeadNeighbors(c, gen, true); 
} 
int calculateNumOfDeadNeighbors(Cell c, int gen) { 
    return calculateNumOfLiveOrDeadNeighbors(c, gen, false); 
} 

OR

あなたはコーディングができ結果クラスまたはint配列を両方の結果を格納するための出力パラメータとして使用します。これは迷惑なブール値のパラメータを取り除くことができます。

+1

私は、パブリックデリゲートメソッドを使用したプライベートメソッドを推奨します。 – heikkim

1

IMO、これはいわゆる「それぞれの方法は一つのことをする」原理を選択的に適用する必要があります。あなたの例は、それを適用しないほうが良いでしょう。戻っていること(可能なプライベートメソッド)を作成します

:Bozhoが言ったように

int countNeighbors(Cell c, int gen, boolean countLive) { 
    int x = c.getX(); 
    int y = c.getY(); 
    int counter = 0; 
    for (int i = x - 1; i <= x + 1; i++) { 
     for (int j = y - 1; j <= y + 1; j++) { 
     if (i == x && j == y) 
      continue; 
     CellState s = getCell(i, j).getCellState(gen); 
     if ((countLive && (s == CellState.LIVE || s == CellState.SICK)) || 
      (!countLive && (s == CellState.DEAD || s == CellState.DEAD4GOOD))) { 
      counter++; 
     } 
     } 
    } 
    return counter; 
} 
1

:むしろ、私はちょうどメソッドの実装ビットを単純化するだろう。しかししかしarroundの他の方法でポイント2と3を組み合わせました両方(生きていると死んだ)とは、(あなたは、ほとんどの場合、生死別々に必要な場合のみ)、結果のうち、死亡または両方を選ぶ二つの方法を追加:

DeadLiveCounter calcLiveAndDead(..) {} 
int calcLive(..) { return calcLiveAndDead(..).getLive; } 
int calcDead(..) { return calcLiveAndDead(..).getDead; } 
1

リファクタリングを使用する点では、できることはいくつかあります。

  • このメソッドをコピーし、2つのバージョンを作成します.1つはtrueをハードコードし、もう1つはfalseをハードコードします。リファクタリングツールは、この定数をインライン化し、必要に応じてコードを削除するのに役立ちます。
  • 下位互換性のために上記のように正しい真/偽のメソッドを呼び出すメソッドを再作成します。この方法をインライン化することができます。
0

は、あなたが現在持っているものの正確なコピーと貼り付けであるプライベートメソッドを持っています。 次に、適切なブール値を持つプライベートメソッドを単に呼び出すよりわかりやすい名前の2つの新しいメソッドを作成します。

1

ここでは、CellState列挙からマップを保持して、LIVEとSICKを追加する必要に応じてDEADとDEAD4GOODを選択します。

int calculateNumOfLiveOrDeadNeighbors(Cell c, int gen, boolean countLiveOnes) { 
    final int x = c.getX(); 
    final int y = c.getY(); 
    final HashMap<CellState, Integer> counts = new HashMap<CellState, Integer>(); 
    for (CellState state : CellState.values()) 
     counts.put(state, 0); 

    for (int i = x - 1; i < x + 2; i++) { 
     for (int j = y - 1; j < y + 2; j++) { 
      if (i == x && j == y) 
       continue; 
      CellState state = getCell(i, j).getCellState(gen); 
      counts.put(state, counts.get(state) + 1); 
     } 
    } 
    if (countLiveOnes) 
     return counts.get(CellState.LIVE) + counts.get(CellState.SICK); 
    else 
     return counts.get(CellState.DEAD) + counts.get(CellState.DEAD4GOOD); 
} 
関連する問題