2

我有以下代码。唯一的问题是我们通过一个 checkstyle 程序运行它，它会出现错误 Cyclomatic Complexity is 11 (max allowed is 10)。我想知道如何删除其中一个 if 语句以使其执行相同的操作并让程序通过测试。

 /**
 * 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 语句），但如果方法的代码适合单个页面，我不会让循环复杂性决定我的编码。在大型项目中，您希望达到的目标是可读性和易于理解。请记住，代码可能只会被编写一次，但会被多次阅读。

于 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语句组成。

您可以将两个或多个if语句与or：

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

应替换为：

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