2

次のコードがあります。唯一の問題は、checkstyle プログラムで実行すると、Cyclomatic Complexity is 11 (最大許容数は 10) というエラーが表示されることです。if ステートメントの 1 つを削除して同じことを行い、プログラムをテストに合格させる方法を知りたいです。

 /**
 * Check if there is a winner on the board
 * @return the winner if BLANK there is no winner
 **/

public char checkWinner(){
   this.winner = BLANK;
   int totalTiles = GRIDSIZE*GRIDSIZE;

    //Check if the game has a win
   for (int i=0; i < GRIDSIZE; i++) {

    if((grid[i][0] == grid[i][1]) && (grid[i][1] == grid[i][2])){
        winner = grid[i][0];
        return winner;
    }
    if((grid[0][i] == grid[1][i]) && (grid[1][i] == grid[2][i])){
        winner = grid[0][i];
        return winner;
    }

   }

   if((grid[0][0] == grid[1][1]) && (grid[1][1] == grid[2][2])){
        winner = grid[0][0];
        return winner;
    }

   if((grid[0][2] == grid[1][1]) && (grid[1][1] == grid[2][0])){
        winner = grid[0][2];
        return winner;
   }
   //Check if the game is a tie

   if (movesMade == totalTiles){
    winner = TIE;
   }
   return winner;
}
4

5 に答える 5

3

チェッカーがどのように機能するかはわかりませんが、これはどうですか？

if(((grid[0][0] == grid[1][1]) && (grid[1][1] == grid[2][2])) || 
   ((grid[0][2] == grid[1][1]) && (grid[1][1] == grid[2][0]))) {
     winner = grid[1][1];
     return winner;
 }

これが機能する場合、皮肉なことに、これはコードよりも少し読みにくいように見えます。

于 2011-05-14T15:25:16.743 に答える
2

行と列をチェックするメソッドを抽出し、コードを次のように書き直すことができます。

public char checkWinner()
{    
   for (int i=0; i < GRIDSIZE; i++) {
       if (checkRow(i)) return winner;
       if (checkColumn(i)) return winner;    
   }

   if (checkDiagTopLeft()) return winner;
   if (checkDiagBottomLeft()) return winner;
}

読みやすく、複雑さが軽減されます。

補足: 明らかに、winner再設計を使用することもできますが、それは質問の一部ではなく、読者 (およびコメント作成者) が気が向いたら演習として残されています。

于 2011-05-14T15:35:56.297 に答える
1

ソリューションはすでにそこにあります (if ステートメントを組み合わせます) が、メソッドのコードが 1 ページに収まる場合、循環的複雑性にコーディングを指示させたくありません。大きなプロジェクトで目指したい尺度は、読みやすさとわかりやすさです。コードは 1 回だけ書き込まれる可能性がありますが、何度も読み取られることに注意してください。

于 2011-05-14T15:31:19.487 に答える
0

最初のステップは、等式から冗長性を取り除くことです。allEqual により、意図が少し明確になります。

勝者をフィールドに割り当てるのは奇妙です。リファクタリングでそれを削除しました。割り当てが本当に必要な場合は、checkWinner を呼び出す別のメソッドで行うことができます。戻り値と代入の問題は、呼び出し元がこの副作用を持つことは予想外であるということです。

public char checkWinner() {
    // Check if the game has a win
    for (int i = 0; i < GRIDSIZE; i++) {
        if (allEqual(grid[i][0], grid[i][1], grid[i][2])) return grid[i][0];
        if (allEqual(grid[0][i], grid[1][i], grid[2][i])) return grid[0][i];
    }

    if (allEqual(grid[0][0], grid[1][1], grid[2][2])) return grid[0][0];
    if (allEqual(grid[0][2], grid[1][1], grid[2][0])) return grid[0][2];

    // Check if the game is a tie
    int totalTiles = GRIDSIZE * GRIDSIZE;
    return movesMade == totalTiles ? TIE : BLACK;
}

private boolean allEqual(char... c) {
    for(int i=1;i<c.length;i++) if(c[i-1] != c[i]) return false;
    return true;
}

未解決の問題:

  • char[][] 配列は、ボードを表す最も効率的なデータ構造ではありません。BitSet を使用できます。
  • GRIDSIZE 定数を定義しましたが、実際に 3 から別の値に変更すると故障する可能性があります。
  • 行/列と対角線のチェックが対称であるという事実を利用できます。これを使用してパラメータを転置する必要があります。

GRIDSIZE 定数を使用すると、すべてのセルを明示的に指定する必要はありません。

public char checkWinner() {
    // Check if the game has a win
    for (int i = 0; i < GRIDSIZE; i++) {
        if (rowEqual(i)) return grid[i][0];
        if (columnEqual(i)) return grid[0][i];
    }

    if (diagonalLeftToRightEqual()) return grid[0][0];
    if (diagonalRightToLefttEqual()) return grid[0][GRIDSIZE];

    // Check if the game is a tie
    int totalTiles = GRIDSIZE * GRIDSIZE;
    return movesMade == totalTiles ? TIE : BLACK;
}

private boolean rowEqual(int r) {
    for(int i=1;i<GRIDSIZE;i++) if(grid[r][i-1] != grid[r][i]) return false;
    return true;
}

private boolean diagonalLeftToRightEqual() {
    for(int i=1;i<GRIDSIZE;i++) if(grid[i-1][i-1] != grid[i][i]) return false;
    return true;
}
于 2011-05-14T15:52:43.570 に答える
-1

サイクロメトリック複雑度は、コードを通るパスの数の尺度です。関数はほとんどifステートメントだけで構成されています。

2 つ以上のifステートメントを で組み合わせることができorます。

if(a)
  do_something();
if(b)
  do_something();

次のものに置き換える必要があります。

if(a || b)
  do_something();
于 2011-05-14T15:26:22.250 に答える