Tic Tac Toe in Android

Question

I made my first Tic Tac Toe game on Android and I'm looking for a way to simplify, make the code more flexible and remove the hardcoded code. I managed to do this game by looking at different tutorials, but I think it could be easier than that. I would like to understand better in the future that I do not have so many lines in future applications.

The game logic is here:

public class PlayGame extends AppCompatActivity {

    int c[][];
    int i, j, k = 0;
    Button b[][];
    TextView textView;
    AI ai;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_play_game);
        setBoard();
    }

    public boolean onOptionsItemSelected(MenuItem item) {
        setBoard();
        return true;
    }

    private void setBoard() {
        ai = new AI();
        b = new Button[4][4];
        c = new int[4][4];


        textView = (TextView) findViewById(R.id.dialogue);

        b[1][3] = (Button) findViewById(R.id.one);
        b[1][2] = (Button) findViewById(R.id.two);
        b[1][1] = (Button) findViewById(R.id.three);


        b[2][3] = (Button) findViewById(R.id.four);
        b[2][2] = (Button) findViewById(R.id.five);
        b[2][1] = (Button) findViewById(R.id.six);


        b[3][3] = (Button) findViewById(R.id.seven);
        b[3][2] = (Button) findViewById(R.id.eight);
        b[3][1] = (Button) findViewById(R.id.nine);

        for (i = 1; i <= 3; i++) {
            for (j = 1; j <= 3; j++)
                c[i][j] = 2;
        }

        textView.setText("Click a button to start.");

        // add the click listeners for each button
        for (i = 1; i <= 3; i++) {
            for (j = 1; j <= 3; j++) {
                b[i][j].setOnClickListener(new MyClickListener(i, j));
                if(!b[i][j].isEnabled()) {
                    b[i][j].setText(" ");
                    b[i][j].setEnabled(true);
                }
            }
        }
    }

    private class MyClickListener implements View.OnClickListener {
        int x;
        int y;

        MyClickListener(int x, int y) {
            this.x = x;
            this.y = y;
        }

        public void onClick(View view) {
            if (b[x][y].isEnabled()) {
                b[x][y].setEnabled(false);
                b[x][y].setText("O");
                c[x][y] = 0;
                textView.setText("");
                if (!checkBoard()) {
                    ai.takeTurn();
                }
            }
        }
    }

    private class AI {
        void takeTurn() {
            if(c[1][1]==2 &&
                    ((c[1][2]==0 && c[1][3]==0) ||
                            (c[2][2]==0 && c[3][3]==0) ||
                            (c[2][1]==0 && c[3][1]==0))) {
                markSquare(1,1);
            } else if (c[1][2]==2 &&
                    ((c[2][2]==0 && c[3][2]==0) ||
                            (c[1][1]==0 && c[1][3]==0))) {
                markSquare(1,2);
            } else if(c[1][3]==2 &&
                    ((c[1][1]==0 && c[1][2]==0) ||
                            (c[3][1]==0 && c[2][2]==0) ||
                            (c[2][3]==0 && c[3][3]==0))) {
                markSquare(1,3);
            } else if(c[2][1]==2 &&
                    ((c[2][2]==0 && c[2][3]==0) ||
                            (c[1][1]==0 && c[3][1]==0))){
                markSquare(2,1);
            } else if(c[2][2]==2 &&
                    ((c[1][1]==0 && c[3][3]==0) ||
                            (c[1][2]==0 && c[3][2]==0) ||
                            (c[3][1]==0 && c[1][3]==0) ||
                            (c[2][1]==0 && c[2][3]==0))) {
                markSquare(2,2);
            } else if(c[2][3]==2 &&
                    ((c[2][1]==0 && c[2][2]==0) ||
                            (c[1][3]==0 && c[3][3]==0))) {
                markSquare(2,3);
            } else if(c[3][1]==2 &&
                    ((c[1][1]==0 && c[2][1]==0) ||
                            (c[3][2]==0 && c[3][3]==0) ||
                            (c[2][2]==0 && c[1][3]==0))){
                markSquare(3,1);
            } else if(c[3][2]==2 &&
                    ((c[1][2]==0 && c[2][2]==0) ||
                            (c[3][1]==0 && c[3][3]==0))) {
                markSquare(3,2);
            }else if( c[3][3]==2 &&
                    ((c[1][1]==0 && c[2][2]==0) ||
                            (c[1][3]==0 && c[2][3]==0) ||
                            (c[3][1]==0 && c[3][2]==0))) {
                markSquare(3,3);
            } else {
                Random rand = new Random();

                int a = rand.nextInt(4);
                int b = rand.nextInt(4);
                while(a==0 || b==0 || c[a][b]!=2) {
                    a = rand.nextInt(4);
                    b = rand.nextInt(4);
                }
                markSquare(a,b);
            }
        }


        private void markSquare(int x, int y) {
            b[x][y].setEnabled(false);
            b[x][y].setText("X");
            c[x][y] = 1;
            checkBoard();
        }
    }

    private void markSquare(int x, int y) {
        b[x][y].setEnabled(false);
        b[x][y].setText("X");
        c[x][y] = 1;
        checkBoard();
    }

    // check the board to see if someone has won
    private boolean checkBoard() {
        boolean gameOver = false;
        if ((c[1][1] == 0 && c[2][2] == 0 && c[3][3] == 0)
                || (c[1][3] == 0 && c[2][2] == 0 && c[3][1] == 0)
                || (c[1][2] == 0 && c[2][2] == 0 && c[3][2] == 0)
                || (c[1][3] == 0 && c[2][3] == 0 && c[3][3] == 0)
                || (c[1][1] == 0 && c[1][2] == 0 && c[1][3] == 0)
                || (c[2][1] == 0 && c[2][2] == 0 && c[2][3] == 0)
                || (c[3][1] == 0 && c[3][2] == 0 && c[3][3] == 0)
                || (c[1][1] == 0 && c[2][1] == 0 && c[3][1] == 0)) {
            textView.setText("Game over. You win!");
            gameOver = true;
        } else if ((c[1][1] == 1 && c[2][2] == 1 && c[3][3] == 1)
                || (c[1][3] == 1 && c[2][2] == 1 && c[3][1] == 1)
                || (c[1][2] == 1 && c[2][2] == 1 && c[3][2] == 1)
                || (c[1][3] == 1 && c[2][3] == 1 && c[3][3] == 1)
                || (c[1][1] == 1 && c[1][2] == 1 && c[1][3] == 1)
                || (c[2][1] == 1 && c[2][2] == 1 && c[2][3] == 1)
                || (c[3][1] == 1 && c[3][2] == 1 && c[3][3] == 1)
                || (c[1][1] == 1 && c[2][1] == 1 && c[3][1] == 1)) {
            textView.setText("Game over. You lost!");
            gameOver = true;
        } else {
            boolean empty = false;
            for(i=1; i<=3; i++) {
                for(j=1; j<=3; j++) {
                    if(c[i][j]==2) {
                        empty = true;
                        break;
                    }
                }
            }
            if(!empty) {
                gameOver = true;
                textView.setText("Game over. It's a draw!");
            }
        }
        return gameOver;
    }
}

Answer 1

Favor enum values over magic values

    int c[][];

This holds the board and the numbers represent state. Consider

    Status board[][];

where

public enum Status {

    X,
    O

}

Then code like

            if(c[1][1]==2 &&
                    ((c[1][2]==0 && c[1][3]==0) ||

would become something like

            if (board[1][1] == null
             && ((board[1][2] == Status.X && board[1][3] == Status.X) 

Then we don't have to sit there hunting through the code to find out that 0 is the player, 1 is the AI, and 2 is not set yet.

A side advantage of using null instead of 2 is that it is the default value. So you don't have to initialize the board explicitly. Java will do it for you implicitly. You could also get this by switching the meaning of 0 and 2, as 0 is the int default. But I like null as "not set yet" better. It better represents the idea that it will be set eventually.

The Status.X and Status.O are much clearer about what they hold. You could also use Status.PLAYER and Status.AI to be even clearer. But then you couldn't reuse the code for two player games.

The name board is more descriptive than c.

Zero-indexed

Arrays in Java are zero-indexed, so instead of

        b = new Button[4][4];
        c = new int[4][4];

It is more common to say

        b = new Button[3][3];
        c = new int[3][3];

Then code like

        b[1][3] = (Button) findViewById(R.id.one);

would change to

        b[0][2] = (Button) findViewById(R.id.one);

And code like

        for (i = 1; i <= 3; i++) {
            for (j = 1; j <= 3; j++)
                c[i][j] = 2;
        }

would become

        for (int i = 0; i < c.length; i++) {
            for (int j = 0; j < c[i].length; j++) {
                c[i][j] = 2;
            }
        }

This will save seven memory locations in this case (using nine rather than sixteen).

This is more idiomatic and recognizable to the typical Java programmer.

This gets rid of the two magic 3 values. Now if you change the size of the board, this would continue to work.

It's more common to declare Java loop counters in the for declaration. You would normally only do it the other way if you planned to use the final values outside of the loop.

One-dimensional data store

Others who have worked with Tic-Tac_Toe have found it easier to work with a one-dimensional data structure. There's a little more work to move from row to row, but it's easier to iterate over the whole structure.

Helper methods

You have almost identical code

        if ((c[1][1] == 0 && c[2][2] == 0 && c[3][3] == 0)
                || (c[1][3] == 0 && c[2][2] == 0 && c[3][1] == 0)
                || (c[1][2] == 0 && c[2][2] == 0 && c[3][2] == 0)
                || (c[1][3] == 0 && c[2][3] == 0 && c[3][3] == 0)
                || (c[1][1] == 0 && c[1][2] == 0 && c[1][3] == 0)
                || (c[2][1] == 0 && c[2][2] == 0 && c[2][3] == 0)
                || (c[3][1] == 0 && c[3][2] == 0 && c[3][3] == 0)
                || (c[1][1] == 0 && c[2][1] == 0 && c[3][1] == 0)) {
            textView.setText("Game over. You win!");
            gameOver = true;

With a helper method, you can write both as

        if (hasWin(Status.X)) {
            textView.setText("Game over. You win!");
            gameOver = true;
        } else if (hasWin(Status.O)) {

And then you just need five methods like

    private boolean hasWin(Status player) {
        for (int row = 0; row < board.length; row += BOARD_WIDTH) {
            if (hasRowWin(player, row)) {
                return true;
            }
        }

        for (int column = 0; column < BOARD_WIDTH; column++) {
            if (hasColumnWin(player, column)) {
                return true;
            }
        }

        return hasForwardDiagonalWin(player) || hasBackwardDiagonalWin(player);
    }

    private boolean hasRowWin(Status player, int row) {
        for (int i = row, n = row + BOARD_WIDTH; i < n; i++) {
            if (board[i] != player) {
                return false;
            }
        }

        return true;
    }

    private boolean hasColumnWin(Status player, int column) {
        for (int i = column; i < board.length; column += BOARD_WIDTH) {
            if (board[i] != player) {
                return false;
            }
        }

        return true;
    }

    private boolean hasForwardDiagonalWin(Status player) {
        for (int i = 0; i < board.length; column += BOARD_WIDTH + 1) {
            if (board[i] != player) {
                return false;
            }
        }

        return true;
    }

    private boolean hasBackwardDiagonalWin(Status player) {
        for (int i = 0, n = board.length - 1; i < n; column += BOARD_WIDTH - 1) {
            if (board[i] != player) {
                return false;
            }
        }

        return true;
    }

These will handle any square board size without further changes. Except to the BOARD_WIDTH variable or constant.

This will not handle diagonals properly for non-square boards. Other solutions do handle that.

You just call these with the player that you want to check. The same code works for both.

You could save a call a turn to either of these by only calling the appropriate one after a player moves. Because if players weren't winning at the end of their turn, they still won't be winning after the other player moves.

Notice how the one-dimensional board means that you can move along the diagonals with simple additions to one variable. A two-dimensional board variable would require two variables to traverse.

Track the number of turns

You check to see if there are any places left after each move. This requires iterating one variable nine times and another three times. But if you tracked a turn count, you could iterate that once for each move. Then you'd check if done by

if (turnCount >= board.length) {

This would also allow you to announce what move it is if you wanted.