How can I remove one of these if statements and shorten the code?

I have the following code. The only problem is that we run it through a checkstyle program and it comes up with the error Cyclomatic Complexity is 11 (max allowed is 10). I would like to know how can remove one of the if statement to make it do the same thing and let the program pass the test.

 /**
 * 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]) &&开发者_开发百科amp; (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;
}

I don't know how the checker works but how about this:

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

If this does work, the irony of course is that this seems a little less readable than your code.

You could extract methods for checking rows and column and rewrite your code something like this:

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

Easier to read and less complexity.

Side note: Obviously, the winner stuff could use a redesign, but that was not part of the question and is left as an exercise for the reader (and commenters) if they feel like it.

The solution is already up there (combining the if statements), but I would not let Cyclomatic Complexity dictate my coding if the code of a method fits on a single page. The measure you want to aim for in a big project is readability and ease of understanding. Remember that code will be written potentially only once, but read quite a few times.

The first step can be to remove some redundancy from the equal expression. The allEqual makes the intent a bit clearer.

Assinging the winner to a field is strange. I've removed that in the refactoring. If you really need the assignment you could do it in a separate method calling checkWinner. The problem with returning and assigning is that it's unexpected for a caller to have this side effect.

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

Open Problems:

• The char[][] array is not the most efficient data structure to represent the board. You could use a BitSet.
• You defined GRIDSIZE constant but you're could would break down if you actually changed it from 3 to another value.
• You can use the fact that checking row/columns and diagonals is symmetric. The parameters have to be transposed use this.

Using the GRIDSIZE constant you do not have to address all cells explicitly:

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

Cyclometric complexity is a measure of the number of paths through your code. Your function is composed almost exclusively of if statements.

You can combine two or more if statements with or:

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

Should be replaced by:

if(a || b)
  do_something();