I'm working on a Tic-tac-toe game.

I've got a method, which initially looked like this:

    static boolean checkWin(char dot) {
        if (map[0][0] == dot && map[0][1] == dot && map[0][2] == dot) {
            return true;
        }

        // more code

        if (map[0][2] == dot && map[1][1] == dot && map[2][0] == dot) {
            return true;
        }

        return false;
    }

With loops, I refactored this method to:

static boolean checkWin(char dot) {
    for (int i = 0; i <map.length ; i  ) {
        for (int j = 0; j < map[i].length; j  ) {
            if (map[0][i] == dot && i == 2) {
                return true;
            }
            if (map[1][i] == dot && i == 2) {
                return true;
            }
            if (map[2][i] == dot && i == 2) {
                return true;
            }
            if (map[i][0] == dot && i == 2) {
                return true;
            }
            if (map[i][1] == dot && i == 2) {
                return true;
            }
            if (map[i][2] == dot && i == 2) {
                return true;
            }

            if (map[i][i] == dot && i == 2) {
                return true;
            }

            if (map[0][2] == dot && map[1][1] == dot && map[2][0] == dot) {
                return true;
            }
        }
    }
    return false;
}

I've got a problem with this line of code:

     if (map[0][2] == dot && map[1][1] == dot && map[2][0] == dot) 
              return true;
     }

I need to get map[i][?].

Where ? - 2, 1, 0

How can I achieve it?

CodePudding user response：

You are checking all the possible combinations on the game field. But it is redundant because you know the coordinates of the last move.

It's sufficient to find whether the cells adjacent with the current cell (the cell of the last move) form a winning combination. In order to do that, you might declare a static variable that will encompass all winning combinations for an arbitrary cell.

And there are only four cases:

• cells are aligned horizontally -;
• cells are aligned vertically |;
• next two cases: cells are forming a diagonal line \ or /.

And this class variable will be a nested array where each element will represent a shift which needs to be added to the current cell in order to find the coordinates of its neighbor:

public static final int[][][] NEIGHBOURS =
                   {{{0, -1}, {0, 1}},   // horizontal -
                    {{-1, 0}, {1, 0}},   // vertical   |
                    {{-1, -1}, {1, 1}},  // diagonal   \
                    {{1, 1}, {-1, -1}}}; // diagonal   /

The method isWinningMove() as its name suggests, checks whether the current move is a winning move. It expects coordinates of the last move and iterates over NEIGHBOURS array. If a winning combination wasn't found, it returns false:

public static boolean isWinningMove(int row, int col) {
    for (int[][] next: NEIGHBOURS) {
        if (isWinningCombo(next, row, col)) {
            return true;
        }
    }
    return false;
}

Method isWinningCombo() validates the combination by checking where the "left" neighbour's value and the "right" neighbour's value are equal to the current value.

public static boolean isWinningCombo(int[][] neighbour, int row, int col) {
    int[] leftShift = neighbour[0];
    int[] rightShift = neighbour[1];

    return getMapValue(row   leftShift[0], col   leftShift[1]) == map[row][col] &&  // leftNeighbour == current
           getMapValue(row   rightShift[0], col   rightShift[1]) == map[row][col];  // rightNeighbour == current
}

Method getMapValue() is responsible for validation of the coordinates of neighbors. If the neighbor doesn't exist it'll return an invalid character so that isWinningCombo() will return false. If the neighbor exists, the method yields its value.

public static char getMapValue(int row, int col) {
    if (row < 0 || row == map.length || col < 0 || col == map.length) { // invalid index
        return '!'; // illegal value for which `isWinningCombo()` will return false
    }
    return map[row][col];
}

Note: all methods are deliberately designed to be narrow-focused, as the Single responsibility principle suggests. It makes it easier to read, test and reuse the code.

CodePudding user response：

If you still want to brute force check using loops:

  static boolean checkWin(char dot) {
    boolean winFound = false;
    // check for horizontal win
    for(int row = 0; row < map.length && !winFound; row  ) {
      winFound = checkRow(row, dot);
    }
    // check for vertical win (assuming all rows have same width)
    for(int col = 0; col < map[0].length && !winFound; col  ) {
      winFound = checkColumn(col, dot);
    }
    // check for diagonal wins (they will not be checked if a win has already been found)
    return winFound || checkDiagonal1(dot) || checkDiagonal2(dot);
  }

  static boolean checkRow(int row, char dot) {
    for(char c : map[row]) {
      if (c != dot) return false;
    }
    return true;
  }

  static boolean checkColumn(int col, char dot) {
    for(int row = 0; row < map.length; row  ) {
      if (map[row][col] != dot) return false;
    }
    return true;
  }
  
  static boolean checkDiagonal1(char dot) {
    // Upper left to bottom right (assumes a SQUARE map)
    for(int row = 0, col = 0; row < map.length; row  , col  ) {
      if (map[row][col] != dot) return false;
    }
    return true;
  }

  static boolean checkDiagonal2(char dot) {
    // Upper right to bottom left (assumes a SQUARE map)
    for(int row = 0, col = (map[0].length - 1); row < map.length; row  , col--) {
      if (map[row][col] != dot) return false;
    }
    return true;
  }