3
\$\begingroup\$

Currently I have a game that does this and then checks for victory conditions in victoryTable

victoryTable[0] = rows[0][0] + rows[0][1] + rows[0][2];
victoryTable[1] = rows[1][0] + rows[1][1] + rows[1][2];
victoryTable[2] = rows[2][0] + rows[2][1] + rows[2][2];
victoryTable[3] = rows[0][0] + rows[1][0] + rows[2][0];
victoryTable[4] = rows[0][1] + rows[1][1] + rows[2][1];
victoryTable[5] = rows[0][2] + rows[1][2] + rows[2][2];
victoryTable[6] = rows[0][0] + rows[1][1] + rows[2][2];
victoryTable[7] = rows[0][2] + rows[1][1] + rows[2][0];

But I am considering changing it to read this way:

for (int i = 0; i < 8; i++ ) {
    for (int n = 0; n < 3; n++) {
        for (int t = 0; t < 3; t++) {
            victoryTable[i] = rows[n][t] + // based on some combination of n,t and 
                                       // if statement to filter out garbage/duplicates, 
                                       // the more I think about this implementation the 
                                       // more complicated it seems.
        }
    }
}

Would this be a step in the wrong direction? It seems like I should put the things that change in a method or loop or something, but it also looks like this will make it harder to read what the code does, and have more potential for bugs (undesired conditions, etc) but also more expandability (if game board became larger in future version, for instance.

The larger context is this: This is part of a method that determines the victory state of a Tic Tac Toe game. In a 3x3 int[][], player X is represented by a 3, player O by a 30. The code in this post adds each column, row, and diagonal, and stores them in an array. Another method loops through that array and if any element equals 90, O wins, if it equals 9, X wins, otherwise nobody wins (yet).

\$\endgroup\$
3
\$\begingroup\$

Without an advanced math degree, I think this is a perfectly reasonable solution. Here's a sample implementation. It's not that complicated. Basically, for every WIN_CONDITION that exists (each of which is a set of coordinates on the board), we check that sequence and see if the Marks are all the same. If so, that player has won.

I don't think that's a step in the wrong direction. It's perfectly logical and functional.

Now if it was a more vast game like Connect Four... looping through each variation might be a better option that explicitly listing each win condition, obviously. But since Tic-Tac-Toe has such a small set, this is fine and makes the code pretty readable.

private static Mark[][] board = new Mark[3][3];

private static final int[][][] WIN_CONDITIONS = {
    //Rows
    {{ 0, 0 }, { 0, 1 }, { 0, 2 }},
    {{ 1, 0 }, { 1, 1 }, { 1, 2 }},
    {{ 2, 0 }, { 2, 1 }, { 2, 2 }},

    //Columns
    {{ 0, 0 }, { 1, 0 }, { 2, 0 }},
    {{ 0, 1 }, { 1, 1 }, { 2, 1 }},
    {{ 0, 2 }, { 1, 2 }, { 2, 2 }},

    //Diagonals
    {{ 0, 0 }, { 1, 1 }, { 2, 2 }},
    {{ 2, 0 }, { 1, 1 }, { 0, 2 }}
};

// other stuff, like an enum for Mark (Mark.X and Mark.O) and the GameState

private GameState checkWinConditions() {

    GameState won = null;

    for(int[][] condition : WIN_CONDITIONS) {
        Mark comparator = board[condition[0][0]][condition[0][1]];
        if(comparator == null) continue;

        won = comparator.getWinState();
        for(int[] coordinate : condition) {
            if(board[coordinate[0]][coordinate[1]] != comparator) {
                won = null;
                break;
            }
        }
        if(won != null) break;
    }
    return won;
}
\$\endgroup\$
1
\$\begingroup\$

You could store the total of each row, column and the diagonals and check against it at each move. See below:

private class Referee {
    private static final int NO_OF_DIAGONALS = 2;
    private static final int MINOR = 1;
    private static final int PRINCIPAL = 0;
    private final int gridSize;
    private final int[] rowTotal;
    private final int[] colTotal;
    private final int[] diagonalTotal;

    private Referee(int size) {
        gridSize = size;
        rowTotal = new int[size];
        colTotal = new int[size];
        diagonalTotal = new int[NO_OF_DIAGONALS];
    }

    private String isGameOver(int x, int y, char symbol, int moveCount) {
        if (isWinningMove(x, y, symbol))
            return symbol + " won the game!";
        if (isBoardCompletelyFilled(moveCount))
            return "Its a Draw!";
        return "continue";
    }

    private boolean isBoardCompletelyFilled(int moveCount) {
        return moveCount == gridSize * gridSize;
    }

    private boolean isWinningMove(int x, int y, char symbol) {
        if (isPrincipalDiagonal(x, y) && allSymbolsMatch(symbol, diagonalTotal, PRINCIPAL))
            return true;
        if (isMinorDiagonal(x, y) && allSymbolsMatch(symbol, diagonalTotal, MINOR))
            return true;
        return allSymbolsMatch(symbol, rowTotal, x) || allSymbolsMatch(symbol, colTotal, y);
    }

    private boolean allSymbolsMatch(char symbol, int[] total, int index) {
        total[index] += symbol;
        return total[index] / gridSize == symbol;
    }

    private boolean isPrincipalDiagonal(int x, int y) {
        return x == y;
    }

    private boolean isMinorDiagonal(int x, int y) {
        return x + y == gridSize - 1;
    }
}

Full source available at: https://github.com/nashjain/tictactoe/tree/master/java

\$\endgroup\$
  • 3
    \$\begingroup\$ When you throw an Exception for control flow, it makes me cringe. throw new GameOverException() in order to end the game is really poor design. The rest of your code is quite nice though, odd method names and implicit publics withstanding. \$\endgroup\$ – asteri Oct 17 '13 at 13:19
  • 1
    \$\begingroup\$ I've fixed the exception bit! \$\endgroup\$ – Naresh Jain Oct 18 '13 at 4:19
0
\$\begingroup\$

This is not a direct answer to your question.

You should probably rethink your design. I guess the different elements of rows[][] have some meaning, so you should define a class with the appropriate member variables which have explicit names.

We don't have much information to go on to tell you how to do this. Also, maybe you should rethink victoryTable[].

\$\endgroup\$
0
\$\begingroup\$

I would split the for loop into four pieces: one for loop for the rows, one for loop for the columns, and two single statements for the NE-SW and NW-SE diagonals. I think that still saves some typing, without making the calculations too complicated.

Imagine trying to make a three-dimensional tictactoe: then you would need 3 for loops (in the x/y/z directions), and 4 diagonals. Still pretty reasonable, and readable, in my opinion. Only at four-dimensional tictactoe would I start doing fancy stuff :-)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.