HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Basic TicTacToe game in java

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javatictactoegamebasic

Problem

I was assigned to create a simple Tic Tac Toe game in java for class, and here is what I got. I would like to know how the code could possibly be shortened without using really advanced topics since I haven't covered them yet. Here goes:

```
import java.util.*;

import javax.swing.JOptionPane;

public class TicTacToe {
enum Turn {PLAYER, COMPUTER};

public static void main(String[] args) {
char[][] gameBoard = {{'1','2','3'},
{'4','5','6'},
{'7','8','9'}};

ArrayList choicesMade = new ArrayList();

Turn turn = Turn.PLAYER;

while(!boardFull(gameBoard)) {
if(turn.equals(Turn.PLAYER)) {
int userChoice = getUserChoice(gameBoard);
if(!placeTaken(userChoice, choicesMade)) {
turn = Turn.COMPUTER;
choicesMade = addToArray(userChoice, choicesMade);
gameBoard = changeBoard(userChoice, gameBoard, 'X');
if(checkHorizontalWin(gameBoard, 'X')|| checkVerticalWin(gameBoard, 'X') || checkDiagonalWin(gameBoard, 'X')) {
JOptionPane.showMessageDialog(null, "You Win!");
break;
}
}
else {
JOptionPane.showMessageDialog(null, "That place is taken! Please try again!");
}
}
if(turn.equals(Turn.COMPUTER)) {
int computerChoice = 1 + (int)(Math.random() * 9);
if(!placeTaken(computerChoice, choicesMade)) {
turn = Turn.PLAYER;
choicesMade = addToArray(computerChoice, choicesMade);
gameBoard = changeBoard(computerChoice, gameBoard, 'O');
displayBoard(gameBoard);
if(checkHorizontalWin(gameBoard, 'O') || checkVerticalWin(gameBoard, 'O') || checkDiagonalWin(gameBoard, 'O')) {

Solution

There's lots of duplication here. Cleaning that up will leave things a lot shorter.

switch(userChoice) {
        case 1:
            gameBoard = updateBoard(gameBoard, 0, 0, characterPlaceholder);
            break;
        case 2:
            gameBoard = updateBoard(gameBoard, 0, 1, characterPlaceholder);
            break;
        case 3:
            gameBoard = updateBoard(gameBoard, 0, 2, characterPlaceholder);
            break;
        case 4:
            gameBoard = updateBoard(gameBoard, 1, 0, characterPlaceholder);
            break;
        case 5:
            gameBoard = updateBoard(gameBoard, 1, 1, characterPlaceholder);
            break;
        case 6:
            gameBoard = updateBoard(gameBoard, 1, 2, characterPlaceholder);
            break;
        case 7:
            gameBoard = updateBoard(gameBoard, 2, 0, characterPlaceholder);
            break;
        case 8:
            gameBoard = updateBoard(gameBoard, 2, 1, characterPlaceholder);
            break;
        case 9:
            gameBoard = updateBoard(gameBoard, 2, 2, characterPlaceholder);
            break;
    }


There's an easy way to get the coordinates out of the board position numbering you're using, with integer division and the mod operator. Integer division is like ordinary division, throwing away the remainder, and the mod operator is like division where we throw away the result and keep the remainder. If we use userChoice / 3 and userChoice % 3, we almost get what we want. Subtracting 1 from userChoice will give us what we want.

Now we can rewrite it in a much shorter way:

int x = (userChoice - 1) / 3;
int y = (userChoice - 1) % 3;
gameBoard = updateBoard(gameBoard, x, y, characterPlaceholder);


Note that even if you didn't see this trick, there are other ways of writing it that are also much shorter. For example, you could have made a pair of arrays that let you look up the corresponding x and y coordinate for a grid position, and ended up with code like this:

gameBoard = updateBoard(gameBoard, x[userChoice], y[userChoice], characterPlaceholder);


This method looks at positions one at a time:

public static boolean checkHorizontalWin(char [][] gameBoard, char character) {
    if(gameBoard[0][0] == character && gameBoard[0][1] == character && gameBoard[0][2] == character) {
        return true;
    }
    else if(gameBoard[1][0] == character && gameBoard[1][1] == character && gameBoard[1][2] == character) {
        return true;
    }
    else if(gameBoard[2][0] == character && gameBoard[2][1] == character && gameBoard[2][2] == character) {
        return true;
    }

    return false;
}


What we're doing, really, is checking each row. So let's do that more explicitly:

public static boolean checkHorizontalWin(char [][] gameBoard, char character) {
        for(int i = 0; i < 3; i++) {
            if(gameBoard[i][0] == character && gameBoard[i][1] == character && gameBoard[i][2] == character) {
                return true;
            }
        }

        return false;
    }


You could go a step further and turn the if statement into a loop as well. That doesn't help us a ton now, but if your instructor suddenly decided he wanted you to code tic-tac-toe for a 5x5 board instead, that would make it easy instead of hard.

In this code, you do the exact same thing for a computer turn as for a player turn, with only a few differences:

if(turn.equals(Turn.COMPUTER)) {
                int computerChoice = 1 + (int)(Math.random() * 9);
                if(!placeTaken(computerChoice, choicesMade)) {
                    turn = Turn.PLAYER;
                    choicesMade = addToArray(computerChoice, choicesMade);
                    gameBoard = changeBoard(computerChoice, gameBoard, 'O');
                    displayBoard(gameBoard);
                    if(checkHorizontalWin(gameBoard, 'O') || checkVerticalWin(gameBoard, 'O') || checkDiagonalWin(gameBoard, 'O')) {
                        JOptionPane.showMessageDialog(null, "Computer Wins!");
                        break;
                    }
                }


The differences are: the character representing a move, the way of changing whose turn it is, the message for a win, and the way of picking the move. You could have a checkForWin() method; all you'd have to tell it is what character to check for and what message to display for a win. You could have a changeTurn() method that would swap whose turn it is. You could have a checkMove() method that would check whether the move was valid.

You could probably make the main loop of the program into about 4 or 5 lines of code if you did all that (or something similar). Of course, it would be calling methods with a few lines of code each, and it would end up doing about the same amount of work, but looking at a couple of lines of code at a time where there's only one thing going on is a lot easier than looking at a couple of dozen lines of code where

Code Snippets

switch(userChoice) {
        case 1:
            gameBoard = updateBoard(gameBoard, 0, 0, characterPlaceholder);
            break;
        case 2:
            gameBoard = updateBoard(gameBoard, 0, 1, characterPlaceholder);
            break;
        case 3:
            gameBoard = updateBoard(gameBoard, 0, 2, characterPlaceholder);
            break;
        case 4:
            gameBoard = updateBoard(gameBoard, 1, 0, characterPlaceholder);
            break;
        case 5:
            gameBoard = updateBoard(gameBoard, 1, 1, characterPlaceholder);
            break;
        case 6:
            gameBoard = updateBoard(gameBoard, 1, 2, characterPlaceholder);
            break;
        case 7:
            gameBoard = updateBoard(gameBoard, 2, 0, characterPlaceholder);
            break;
        case 8:
            gameBoard = updateBoard(gameBoard, 2, 1, characterPlaceholder);
            break;
        case 9:
            gameBoard = updateBoard(gameBoard, 2, 2, characterPlaceholder);
            break;
    }
int x = (userChoice - 1) / 3;
int y = (userChoice - 1) % 3;
gameBoard = updateBoard(gameBoard, x, y, characterPlaceholder);
gameBoard = updateBoard(gameBoard, x[userChoice], y[userChoice], characterPlaceholder);
public static boolean checkHorizontalWin(char [][] gameBoard, char character) {
    if(gameBoard[0][0] == character && gameBoard[0][1] == character && gameBoard[0][2] == character) {
        return true;
    }
    else if(gameBoard[1][0] == character && gameBoard[1][1] == character && gameBoard[1][2] == character) {
        return true;
    }
    else if(gameBoard[2][0] == character && gameBoard[2][1] == character && gameBoard[2][2] == character) {
        return true;
    }

    return false;
}
public static boolean checkHorizontalWin(char [][] gameBoard, char character) {
        for(int i = 0; i < 3; i++) {
            if(gameBoard[i][0] == character && gameBoard[i][1] == character && gameBoard[i][2] == character) {
                return true;
            }
        }

        return false;
    }

Context

StackExchange Code Review Q#119949, answer score: 5

Revisions (0)

No revisions yet.