2017-09-21 10 views
0
public boolean hasCapturableEnemy(Piece[][] board, int startX, int startY, int endX, int endY) { 

    //If the Pawn belong to the Upper Team (Faces downward) 
    if(board[startX][startY] != null && board[startX][startY].getTeam() == Player.UP) { 
     //If a Piece exists on a diagonally adjacent tile, return true 
     if(startX - endX == -1 && Math.abs(startY - endY) == 1) { 
      if((board[startX + 1][startY - 1] != null && board[startX + 1][startY - 1].getTeam() != Player.UP) || 
        (board[startX + 1][startY + 1] != null && board[startX + 1][startY + 1].getTeam() != Player.UP)) { 
       return true; 
      } 
     } 
    } 

    if(board[startX][startY] != null && board[startX][startY].getTeam() == Player.DOWN) { 
     //If the Pawn belongs to the Down Team (Faces upward) 

     if(startX - endX == 1 && Math.abs(startY - endY) == 1) { 
      //If a Piece exists on a diagonally adjacent tile, return true 
      if((board[startX - 1][startY - 1] != null && board[startX - 1][startY - 1].getTeam() != Player.DOWN) || 
        (board[startX - 1][startY + 1] != null && board[startX - 1][startY + 1].getTeam() != Player.DOWN)) { 
       return true; 
      } 
     } 
    } 

    return false; 

} 

リファクタリングが難しい2つの同様のif文を含むこの関数があります。少し異なる値を持つ2つの同様のifループをリファクタリングする(Java)

これらの2つのコードは重複したコードのかなりの部分を共有していますが、ボード[startX + 1] [startY-1]とボード[startX-1] [startY-1]私はそれを効率的にリファクタリングするのは難しいと思う。

つまり、一般的な部分を含む別の関数を作成することで確実にこれをリファクタリングできますが、その関数内にループをいくつか作成してコードを汚れさせるのは怖いです。

この種のコードをリファクタリングするためのアドバイスはありますか?

+0

このコードが機能する場合は、[codereview.se]に問い合わせてください。 –

+1

if文はループではありません – JackVanier

+0

チェックの順序を変更することで、if(startX - endX == 1 && Math.abs(startY - endY)== 1){'を一度だけチェックすることができます。 –

答えて

1

「上向き」に応じて乗数変数を1または-1に設定し、「X」値をオフセットするときは常にその変数に乗算します。

if(board[startX][startY] == null) { 
    return false; 
} 

Player player = board[startX][startY].getTeam() 
int xOffset = player == Player.UP 
    ? 1 
    : -1; 
//If a Piece exists on a diagonally adjacent tile, return true 
if(endX - startX == xOffset && Math.abs(startY - endY) == 1) { 
    if((board[startX + xOffset][startY - 1] != null && board[startX + xOffset][startY - 1].getTeam() != player) || 
      (board[startX + xOffset][startY + 1] != null && board[startX + xOffset][startY + 1].getTeam() != player)) { 
     return true; 
    } 
} 

をとにもきれいなものを作るために変数にboard[startX + xOffset]をキャプチャあなたができるいくつかのさらなるリファクタリングは、あります。このような

何かが、動作するはずだと思います。

関連する問題