4
\$\begingroup\$

I recently started learning Java and decided to create a project to practice. I developed a console-based tic-tac-toe game. I also wrote tests for some of the classes, not because they were necessary, but because I wanted to practice writing them.

I would love to hear your thoughts on the project. What can be improved, simplified, or made more efficient?

Here’s the link to the GitHub repository: https://github.com/qwert312/Tic-Tac-Toe-Console-game.git

Code (does not include test classes):

//Class Board3x3

import java.util.Arrays;
import java.util.stream.IntStream;

public class Board3x3 {
    private String[][] board = new String[3][3];

    public Board3x3() {
        resetBoard();
    }

    public String[][] copyOfBoard() {
        return Arrays.stream(board).map(String[]::clone).toArray(String[][]::new);
    }

    public void resetBoard() {
        board = Arrays.stream(board)
                .map(row -> row = new String[]{" ", " ", " "})
                .toArray(String[][]::new);
    }

    public void setSymbol(String symbol, int row, int col) throws IllegalArgumentException {
        if ((row >= board.length || row < 0 || col >= board.length || col < 0)
                || !board[row][col].equals(" "))
            throw new IllegalArgumentException();

        board[row][col] = symbol;
    }

    public boolean isFull() {
        return Arrays.stream(board).allMatch(row -> Arrays.stream(row).noneMatch(" "::equals));
    }

    public String checkWinnerSymbol() {
        String checkRowsResult = checkRows();
        if (!checkRowsResult.equals(" "))
            return checkRowsResult;

        String checkColumnsResult = checkColumns();
        if (!checkColumnsResult.equals(" "))
            return checkColumnsResult;

        String checkDiagonalResult = checkDiagonals();
        if (!checkDiagonalResult.equals(" "))
            return checkDiagonalResult;

        return " ";
    }

    private String checkRows() {
        return Arrays.stream(board)
                .map(row -> checkLine(Arrays.stream(row).toArray(String[]::new)))
                .filter(winner -> !winner.equals(" ")).findFirst().orElse(" ");
    }

    private String checkColumns() {
        return IntStream
                .range(0, board.length)
                .mapToObj(column ->
                        checkLine(Arrays.stream(board)
                                .map(row -> row[column])
                                .toArray(String[]::new)))
                .filter(winner -> !winner.equals(" ")).findFirst().orElse(" ");
    }

    private String checkDiagonals() {
        return IntStream
                .range(0, 2)
                .mapToObj(diagonal ->
                        checkLine(IntStream.range(0, board.length).mapToObj((ind) -> {
                                    if (diagonal == 0)
                                        return board[ind][ind];
                                    else
                                        return board[ind][board.length - 1 - ind];
                                })
                                .toArray(String[]::new)))
                .filter(winner -> !winner.equals(" ")).findFirst().orElse(" ");
    }

    private String checkLine(String[] line) {
        long xCount = Arrays.stream(line).filter("X"::equals).count();
        long oCount = Arrays.stream(line).filter("O"::equals).count();

        if (xCount == 3) return "X";
        if (oCount == 3) return "O";

        return " ";
    }

    public String toString() {
        StringBuilder stringBoard = new StringBuilder();

        for (int row = 0; row < board.length; row++) {
            stringBoard.append(String.join("|", board[row]));
            if (row != board.length - 1)
                stringBoard.append("\n-+-+-\n");
        }

        return stringBoard.toString();
    }
}
//Class Player

public class Player {
    private String symbol;

    public Player(String symbol) {
        this.symbol = symbol;
    }

    public String getSymbol() {
        return symbol;
    }

    public void makeMove(Board3x3 board, int row, int col) throws IllegalArgumentException {
        board.setSymbol(symbol, row, col);
    }

    public String toString() {
        return "Player (" + symbol + ")";
    }
}
//Class AI

import java.util.ArrayList;
import java.util.List;
import java.util.Random;
import java.util.stream.IntStream;

public class AI {
    private String symbol;

    public AI(String symbol) {
        this.symbol = symbol;
    }

    public String getSymbol() {
        return symbol;
    }

    public List<Integer> makeMove(Board3x3 board) {
        List<Integer> randomCoordinates = getRandomCoordinates(board);
        board.setSymbol(symbol, randomCoordinates.get(0), randomCoordinates.get(1));
        return randomCoordinates;
    }

    private List<Integer> getRandomCoordinates(Board3x3 board) {
        Random rand = new Random();
        List<List<Integer>> possibleCoordinates = new ArrayList<>();
        String[][] boardCopy = board.copyOfBoard();

        IntStream.range(0, boardCopy.length)
                .boxed()
                .flatMap(rowInd ->
                        IntStream
                                .range(0, boardCopy[rowInd].length)
                                .filter(colInd -> boardCopy[rowInd][colInd].equals(" "))
                                .mapToObj(colInd -> List.of(rowInd, colInd)))
                .forEach(possibleCoordinates::add);

        return possibleCoordinates.get(rand.nextInt(0, possibleCoordinates.size()));
    }

    public String toString() {
        return "AI (" + symbol + ")";
    }
}
//Class GameManager

import java.util.List;
import java.util.Scanner;

public class GameManager {
    private final Scanner scan;
    private Player player;
    private AI ai;
    private final Board3x3 board = new Board3x3();

    public GameManager(Scanner scanner) {
        this.scan = scanner;
    }

    public void start() {
        System.out.println("TicTacToe console game");

        typePlayerSymbol();

        System.out.println();

        while (true) {
            System.out.println("Start! Type coordinates on your turn in this format: row,column\n");

            playGame();

            System.out.print("Next match! (type n to stop) ");
            String answer = scan.nextLine();
            if (answer.equals("n")) break;
            System.out.println();
            board.resetBoard();
        }
    }

    private void typePlayerSymbol() {
        while (true) {
            System.out.print("Choose player symbol (X/O): ");
            String selectedSymbol = scan.nextLine().trim().toUpperCase();
            try {
                if (selectedSymbol.equals("X")) {
                    player = new Player(selectedSymbol);
                    ai = new AI("O");
                    break;
                } else if (selectedSymbol.equals("O")) {
                    player = new Player(selectedSymbol);
                    ai = new AI("X");
                    break;
                }

                throw new IllegalArgumentException();
            } catch (IllegalArgumentException e) {
                System.out.println("Illegal symbol! Try again");
            }
        }
    }

    private void playGame() {
        while (true) {
            for (int i = 0; i < 2; i++) {
                if ((player.getSymbol().equals("X") && i == 0)
                        || (player.getSymbol().equals("O") && i == 1))
                    playerTurn();
                else
                    aiTurn();

                System.out.println(board.toString() + "\n");
                String winnerSymbol = board.checkWinnerSymbol();
                if (!winnerSymbol.equals(" ")) {
                    String winner = player.getSymbol().equals(winnerSymbol)
                            ? player.toString() : ai.toString();
                    System.out.println("Winner winner chicken dinner. " + winner + " won!");
                    return;
                }

                if (board.isFull()) {
                    System.out.println("Board is full. Draw!");
                    return;
                }
            }
        }
    }

    private void playerTurn() {
        while (true) {
            System.out.println(player + " turn");
            System.out.print("Coordinates: ");

            try {
                String[] coordinates = scan.nextLine().split(",");
                int row = Integer.parseInt(coordinates[0]) - 1;
                int column = Integer.parseInt(coordinates[1]) - 1;
                player.makeMove(board, row, column);
                break;
            } catch (Exception e) {
                System.out.println("Illegal coordinates! Try again");
            }
        }
    }

    private void aiTurn() {
        System.out.println(ai + " turn");
        System.out.print("Coordinates: ");

        List<Integer> coordinates = ai.makeMove(board);

        System.out.println((coordinates.get(0) + 1) + "," + (coordinates.get(1) + 1));
    }
}
//Class Program

import java.util.Scanner;

public class Program {
    public static void main(String[] args) {
        Scanner scan = new Scanner(System.in);

        GameManager gameManager = new GameManager(scan);
        gameManager.start();
    }
}

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Too tiny to be an answer, but GameManager.start should rather be called run, runLoop or smth like that - start conveys an undesired meaning of "there's a stop to be called later", similarly to open that mandates corresponding close. \$\endgroup\$
    – STerliakov
    Commented Aug 10 at 16:53
  • \$\begingroup\$ Why is the board a 2D array of String? It seems these strings are always 1 character long, so why not char[3][3]? \$\endgroup\$
    – k314159
    Commented Aug 14 at 16:45

1 Answer 1

7
\$\begingroup\$

Your names and structuring is already quite good, that you wanted to split logic into these classes, too.

I have to say, I'm not a fan of Streams, I believe they are overhyped and rarely reduce complexity/cognitive load, so take my opinions on these code sections with a grain of salt.


    private String[][] board = new String[3][3];

You might consider using new String[9] instead for your internal representation. That trades a little bit of complexity when setting (board[row * 3 + column]) for easier handling for the most part.


    public String[][] copyOfBoard() {
        return Arrays.stream(board).map(String[]::clone).toArray(String[][]::new);
    }

This doesn't return "a copy of the board" but of "the internal state used by this Board instance" which are two different things. Consider returning a new Board instance instead (as a real copy), or instead provide means to access the state of the board in a readonly fashion, for example through String getSymbol(int row, int column).

The stream logic looks complicated for what it does, but most of that is because you're using a two-dimensional array. If you'd use a one-dimensional array this would boil down to return board.clone().


    public void resetBoard() {
        board = Arrays.stream(board)
                .map(row -> row = new String[]{" ", " ", " "})
                .toArray(String[][]::new);
    }

You're creating a new String[][] object here, but the array has already been allocated. Also, you're declaring the array twice, once with dimensions and once here, which might or might not diverge at some point in the future.

Either remove the initialization with the declaration, or use the existing object here, for example:

for (String[] row : board) {
    for (int index = 0; index < row.length(); index++) {
        row[index] = " ";
    }
}

        if ((row >= board.length || row < 0 || col >= board.length || col < 0)
                || !board[row][col].equals(" "))
            throw new IllegalArgumentException();

You should split this into an IllegalArgumentException and something like IllegalMoveException, both of these with meaningful messages, of course.

That said, you're never preventing someone from setting the symbol "tic-tac-toe sucks" into the board. Consider using an enum for the field content instead.


        String checkRowsResult = checkRows();
        if (!checkRowsResult.equals(" "))
            return checkRowsResult;

You're using " " as "empty" throughout the whole program, consider packing this into an enum or at least a constant to make the code easier to read.

        String checkRowsResult = checkRows();
        if (!checkRowsResult.equals(EMPTY_FIELD))
            return checkRowsResult;
        String checkRowsResult = checkRows();
        if (!checkRowsResult.equals(Symbol.EMPTY))
            return checkRowsResult;

Most of the check logic could be simplified considering that you have harcoded dimensions of 3x3. I have written an example implementation a while ago, unfortunately for you the example is in German, but I guess the code for checking the winner should be understandable regardless.

The gist is, given that you only have 3x3, you can easily write a helper function which checks whether three given fields are identical, and then use that instead.

if (isSamePlayer(topLeft, TopCenter, TopRight)
        || isSamePlayer(middleLeft, middleCenter, middleRight)) {
}

And so forth. Which might be easier to maintain and understand then the nested stream logic.


            stringBoard.append(String.join("|", board[row]));

You're sorta skipping the StringBuilder here by using join, which itself will create a new StringBuilder instance to join the fields together, then return the String, which then will be added to your StringBuilder.


    public void makeMove(Board3x3 board, int row, int col) throws IllegalArgumentException {
        board.setSymbol(symbol, row, col);
    }

The Player class should not have this logic, your game logic should.

With this the Player needs to know about the Board being used, which can be bad and limits the reusability of the Player class somewhat.


public class AI {

Consider making the AI an extension of Player. That way you have two Players and one of them happens to be controlled by a human, and the other by your logic.

I'm a little bit torn whether the AI logic should be here or in the Game class itself. Can't comment on that right now.


    private final Scanner scan;

Don't shorten names just because you can! It makes the code harder to read and harder to maintain.


        typePlayerSymbol();

This should be askPlayerForSymbol or similar.


                throw new IllegalArgumentException();
            } catch (IllegalArgumentException e) {
                System.out.println("Illegal symbol! Try again");
            }

Woof this hurts. You don't need any of that logic, just the println suffices at that point.


            for (int i = 0; i < 2; i++) {
                if ((player.getSymbol().equals("X") && i == 0)
                        || (player.getSymbol().equals("O") && i == 1))
                    playerTurn();
                else
                    aiTurn();

Instead you could store whose the next player, quick and dirty for example:

if (humanIsNext) {
    humanTurn();
} else {
    aiTurn();
}

humanIsNext = !humanIsNext;

Or if you have a common base class for the players, you could also us a List or array and toggle through that.


                System.out.println(board.toString() + "\n");

I missed that above, but consider separating representation of the board for the command line from the Board itself. Instead, your GameManager should know how to format the board (as it deals with the command line).


The Program.main looks exactly as I would expect it to, very nice.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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