Как я могу удалить один из этих операторов if и сократить код?

Question

У меня есть следующий код. Единственная проблема заключается в том, что мы запускаем ее через программу с контрольным стилем, и появляется ошибка Cyclomatic Complexity: 11 (максимально допустимое значение - 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;
}

Answer 1

Я не знаю, как работает средство проверки, но как насчет этого:

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;
 }

Если это работает, ирония, конечно, заключается в том, что это кажется немного менее читабельным, чем ваш код.

Answer 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 может использовать редизайн, но это не было частью вопроса и оставлено в качестве упражнения для читателя (и комментаторов), если они захотят.

Answer 3

Решение уже здесь (объединение операторов if), но я бы не позволил Cyclomatic Complexity диктовать мою кодировку, если код метода помещается на одной странице. Мера, к которой вы хотите стремиться в большом проекте - это удобочитаемость и простота понимания. Помните, что код будет написан потенциально только один раз, но читайте довольно много раз.

Answer 4

Первым шагом может быть удаление некоторой избыточности из одинакового выражения.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;
}

Answer 5

Циклометрическая сложность - это мера количества путей в вашем коде. Ваша функция состоит почти исключительно из if операторов.

Вы можете объединить два или более if операторов с or:

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

Следует заменить на:

if(a || b)
  do_something();