Tic-tac-toe Design

[Campbell Ritchie]

That looks good.
If you think you are not using the Player#play() method, change it to private access or comment it out completely and see whether you get any errors.

[Prasanna Raman]

Thank you. I'll start coding my Game class then. So, my Player class as it stands right now just serves to generate Player objects and doesn't do much else.

[Campbell Ritchie]

What about a record wins method in Player?

[Prasanna Raman]

Sorry, I'm away today and tomorrow. Will post in about 24 hours.

[Prasanna Raman]

Here is the structure of my Game class. Please review.

public class Game {

    private Player[] players;
    private static final int MAX_PLAYERS = 2;
    private Board board;

    public Game() {
        players = new Player[MAX_PLAYERS];
        for (int i = 0; i < MAX_PLAYERS; i++) {
            players[i] = new Player(Symbol.values()[i]);
        }
    }
  /**
    *
    * start() gets the board size from the users, initializes the board
    * with the specified size, or with the default size if the user doesn't 
    * enter a size, and calls the play() method.
    * 
    * @exception NumberFormatException occurs when user enters a bad board size. 
    */
    public void start() { }
 /**
   *
   * getCurrentPlayer() returns the symbol/player that has to take the next 
   * turn.
   * @return Symbol
   */
    private Symbol getCurrentPlayer() { }

 /**
   *
   * play() displays the board state before every turn, determines turns and
   * gets the input for placing the player's symbol on the board and 
   * updates the board. After a turn is completed by updating the board, calls
   * checkGameStatus() to verify if the game has been won or if it has ended in
   * a tie or if it has to continue. If there's a result, the game is ended and the
   * result is displayed.
   * 
   * @exception IllegalArgumentException occurs if the
   * row or column entered is greater than the size of the board.
   * 
   * @exception IllegalMoveException occurs if an already occupied square is
   * mistakenly chosen.
   * 
   */
    public void play() {}
 /**
   *
   * checkGameStatus() determines if the game has been won by calling 
   * isSameInRow(),isSameInColumn(),isSameInBend() and isSameInBendSinister() 
   * or if it has ended in a tie or if it has to continue.
   * 
   * @param Symbol symbol of the player that made the last move.
   * @param row the row of the last played turn, eg. 'a','b','c' and so on.
   * @param col the column of the last played turn, eg. 1,2,3 and so on.
   * 
   * @return GameStatus
   */
    private GameStatus checkGameStatus(Symbol s,char row,int col) {}

And here's my GameStatus enum:

public enum GameStatus {
    WIN,TIE,ONGOING;
}

[Prasanna Raman]

Campbell Ritchie wrote:What about a record wins method in Player?

Should I have player name and other details about the players and write them to a file along with their win/loss count every time to achieve this?

[Campbell Ritchie]

You can do that if you wish. I would suggest that as a later enhancement, though.

[Prasanna Raman]

Thank you. If you feel my Game class is OK, I will start working on the implementation.

[Prasanna Raman]

The above Game class I've written doesn't make use of the play() method in the Player class. So I have just removed it for now. Can I now work on the implementation of the Game class?

[Campbell Ritchie]

Work on the game and see what happens.

[Prasanna Raman]

Here is my Player class:

package game;

public class Player {
    
    private Symbol symbol;
    
    public Player(Symbol symbol) {
        this.symbol = symbol;
    }
    
}

[Prasanna Raman]

Here is the implementation of my Game class. Please review and comment.

package game;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;

public class Game {

    private Player[] players;
    private static final int MAX_PLAYERS = 2;
    private Board board;

    public Game() {
        players = new Player[2];
        for (int i = 0; i < MAX_PLAYERS; i++) {
            players[i] = new Player(Symbol.values()[i]);
        }
    }
  /**
    *
    * start() gets the board size from the users, initializes the board
    * with the specified size, or with the default size if the user doesn't 
    * enter a size, and calls the play() method.
    * 
    * @exception NumberFormatException occurs when user enters a bad board size. 
    */
    public void start() {
        System.out.println("Please enter the board size or press enter to use"
                + " default board size");
        BufferedReader br = new BufferedReader(new InputStreamReader(
                System.in));
        try {
            String pressedKey = br.readLine();
            board = pressedKey.equals("") ? new Board() : new Board(Integer.parseInt(
                    pressedKey));
            play();
        } catch (IOException e) {
            System.err.println(e.getMessage());
        } catch (NumberFormatException nfe) {
            System.err.println("Please enter a valid number");
            start();
        }
    }
/**
   *
   * getCurrentPlayer() returns the symbol/player that has to take the next 
   * turn.
   * @return Symbol
   */
    private Symbol getCurrentPlayer() {
        return (board.getSquareCount() % 2 == 0) ? Symbol.O
                : Symbol.X;
    }

/**
   *
   * play() displays the board state before every turn, determines turns and
   * gets the input for placing the player's symbol on the board and 
   * updates the board. After a turn is completed by updating the board, calls
   * checkGameStatus() to verify if the game has been won or if it has ended in
   * a tie or if it has to continue. If there's a result, the game is ended and the
   * result is displayed.
   * 
   * @exception IllegalArgumentException occurs if BLANK is passed or if the
   * row or column entered is greater than the size of the board.
   * 
   * @exception IllegalMoveException occurs if an already occupied square is
   * mistakenly chosen.
   * 
   */
    public void play() {
        board.displayState();
        Symbol currentPlayer = getCurrentPlayer();
        System.out.println(currentPlayer + "'s turn. Please enter a "
                + "location, for eg. a1 for top left");
        BufferedReader br = new BufferedReader(new InputStreamReader(
                System.in));
        try {
            String location = br.readLine();
            char row = location.charAt(0);
            int col = Integer.parseInt(location.substring(1));          
            board.update(row,col,currentPlayer);
            GameStatus gameStatus = checkGameStatus(currentPlayer,row,col);
            if(gameStatus == GameStatus.WIN) {
                System.out.println("Congratulations!! "+currentPlayer+" "
                        + "wins!!");
            }
            else if(gameStatus == GameStatus.TIE) {
                System.out.println("It's a tie!!");
            }
            else {
                play();
            }
        } catch (IOException ex) {
            System.err.println("An error occured");
        } catch (IllegalMoveException | IllegalArgumentException ex) {
            System.err.println(ex.getMessage());
            play();
        }   
    }
/**
   *
   * checkGameStatus() determines if the game has been won by calling 
   * isSameInRow(),isSameInColumn(),isSameInBend() and isSameInBendSinister() 
   * or if it has ended in a tie or if it has to continue.
   * 
   * @param Symbol symbol of the player that made the last move.
   * @param row the row of the last played turn, eg. 'a','b','c' and so on.
   * @param col the column of the last played turn, eg. 1,2,3 and so on.
   * 
   * @return GameStatus
   */
    private GameStatus checkGameStatus(Symbol s,char row,int col) {
        if(board.isSameInRow(s, row) || board.isSameInColumn(s, col) || 
                board.isSameInBend(s, row, col) || 
                board.isSameInBendSinister(s, row, col)) {
            return GameStatus.WIN;
        }
        else if(board.getSquareCount() == board.getSize() * board.getSize()) {
            return GameStatus.TIE;
        }
        else {
            return GameStatus.ONGOING;
        }
    }
    public static void main(String[] args) {
        Game game = new Game();
        game.start();
    }
}

[Campbell Ritchie]

Not at all, I am afraid.
Why are you calling methods recursively from inside catch blocks? That looks like very poor design.
Why are you repeatedly creating reader objects and allowing them to go out of scope?
Why are you swapping 2 and MAX_PLAYERS? Why have you got a for loop without .length?
Why are you using a reader rather than a Scanner?

[Prasanna Raman]

Thank you for reviewing. Sorry, I'll work on these About the recursive calls from catch block, I just thought of one scenario. Will the program run out of space if it keeps throwing exceptions because the methods below never stop? Is there any other reason why it shouldn't be done?

[Campbell Ritchie]

You are using exceptions instead of ordinary control structures.
Search my posts for Scanner and hasNextInt and nextint and you will see (as Rob Spoor told me ages ago) that you can run a loop which can be guaranteed to return an int from the keyboard. Look here as well as searching. That means you can be sure there is no need to do any exception handling.
There is a much simpler way to keep the people taking turns. When you play noughts and crosses, you don't stop because an Exception occurs, do you? Think when you stop and when you continue.

[Prasanna Raman]

Thank you. I'll search through the old posts. In general, you said calling a method from a catch block is poor design because it might result in the program running out of memory if the exception keeps happening? Is that the (only) reason or are there other issues that might come out of it?

[Campbell Ritchie]

Prasanna Raman wrote: . . . you said calling a method from a catch block is poor design because it might result in the program running out of memory . . .

No, I didn't.

[Prasanna Raman]

Sorry! I just meant to ask why you said it (calling methods recursively from inside catch blocks) was bad design.

[Prasanna Raman]

I wrote this utility class:

public class MyUtilities {

    private static final Scanner INPUT_SCANNER = new Scanner(System.in);

    private MyUtilities() {

    }

  public static int getIntFromScanner(String errorMessage) {
        while ((!INPUT_SCANNER.hasNextInt())) {
            System.err.println(errorMessage);
            INPUT_SCANNER.next();
        }
        return INPUT_SCANNER.nextInt();
    }
  }

Then I've modified my start() method in the Game class as below:

/**
    *
    * start() gets the board size from the users, initialises the board
    * with the specified size, or with the default size if the user 
    * enters 0, and then calls the play() method. 
    */
    public void start() {
            int boardSize = -1;
        System.out.println("Please enter the board size or press 0 to use"
                + " default board size");    
       while(boardSize < 3 && boardSize != 0) {
           boardSize = MyUtilities.getIntFromScanner(""
                    + "Please enter an integer greater than 2"
                    + " or enter 0.");
       }             
            board = (boardSize == 0) ? new Board() : new Board(boardSize);
            play(); 
    }

Does this look OK?

[Prasanna Raman]

I am still thinking about how to call the same method again when I encounter an exception, but I think I'm going nowhere Could you please give me a hint?

[Campbell Ritchie]

Prasanna Raman wrote: . . . Does this look OK?

Yes, that looks good, except for two tiny things.
You have one unnecessary pair of () in the utility class.
You should say default value of 3. I suggest you set a maximum size for the board, unless you want to play on a 97×97 board.


You realise you can enhance the utility class to return a number in a particular range?

[Campbell Ritchie]

If you play the game with a real pencil and paper, how do you tell whether you need to take more turns? You have actually said that, or something similar, a long time ago.

[Prasanna Raman]

Campbell Ritchie wrote:I suggest you set a maximum size for the board, unless you want to play on a 97×97 board.

I definitely don't want to play on a 97*97 board I'll set a maximum size.

Campbell Ritchie wrote:You realise you can enhance the utility class to return a number in a particular range?

Sorry if I'm mistaken, but do you mean I can enhance the utility class to make it return a number less than the maximum size of the board? If so, should I not do that in my application (Game class)?

[Prasanna Raman]

Here is my updated MyUtilites class:

package game;

import java.util.Scanner;

public class MyUtilities {

    private static final Scanner INPUT_SCANNER = new Scanner(System.in);

    private MyUtilities() {

    }

    /**
     *
     * @param errorMessage
     * @return
     */
    public static int getIntFromScanner(String errorMessage) {
        while (!INPUT_SCANNER.hasNextInt()) {
            System.err.println(errorMessage);
            INPUT_SCANNER.next();
        }
        return INPUT_SCANNER.nextInt();
    }

    public static String getLine() {
        String line="";
        while (line.trim().isEmpty()) {
            line = INPUT_SCANNER.nextLine();
        }
        return line;
    }
}

And my Game class:

package game;

public class Game {

    private final Player[] players;
    private static final int MAX_PLAYERS = 2;
    private static final int MAX_BOARD_SIZE = 9;
    private GameStatus gameStatus;
    private Board board;

     /**
     *
     * Game() creates 2 players and initialises them with their symbols
     */
    public Game() {
        players = new Player[MAX_PLAYERS];
        for (int i = 0; i < players.length; i++) {
            players[i] = new Player(Symbol.values()[i]);
        }
    }

    /**
     *
     * start() gets the board size from the users, initialises the board with
     * the specified size, or with the default size of 3 if the user enters 0.
     */
    public void start() {
        int boardSize = -1;
        
        while((boardSize < 3 && boardSize != 0) || boardSize > MAX_BOARD_SIZE) 
        {
            System.out.println("Please enter a board size less than "
                + MAX_BOARD_SIZE + " or press 0 to use the "
                + "default size of 3");
            boardSize = MyUtilities.getIntFromScanner(""
                    + "Please enter an integer between than 3 and " 
                    + MAX_BOARD_SIZE + " or enter 0.");
        }
        board = (boardSize == 0) ? new Board() : new Board(boardSize);
        run(); 
    }
    
    /**
     *
     * run() displays the board state before every turn, determines turns, gets
     * the input from the player and calls play(). After play() returns, checks
     * if the game has been won or if it has ended in a tie or if it has to
     * continue. If there's a result, the game is ended and the result is
     * displayed. If not, the game is continued.
     *
     *
     */
    private void run() {
        while (gameStatus != GameStatus.WIN && gameStatus != GameStatus.TIE) {
            board.displayState();
            Symbol currentPlayer = getCurrentPlayer();
            System.out.println(currentPlayer + "'s turn. "
                    + "Please enter a location, for eg. a1 for top left");
            String location = MyUtilities.getLine();
 
            while(!location.substring(1).matches(
                    "\\d{1,}")){
                System.err.println("Please enter a valid location");
                location = MyUtilities.getLine();
            }
            char row = location.charAt(0);
            int col = Integer.parseInt(location.substring(1));
            if (play(row, col, currentPlayer)) {
                gameStatus = checkGameStatus(currentPlayer, row, col);
                if (gameStatus == GameStatus.WIN) {
                    board.displayState();
                    System.out.println("Congratulations!! " + 
                            currentPlayer + " "
                            + "wins!!");
                } else if (gameStatus == GameStatus.TIE) {
                    board.displayState();
                    System.out.println("It's a tie!!");
                }
            }
        }
    }
    
    /**
     *
     * play() gets the location and symbol from run() and calls the update() 
     * method in board.
     * 
     * @param row the row where the user's symbol has to be placed.
     * @param col the column where the user's symbol has to be placed.
     * @Symbol currentPlayer the  current symbol to be placed. 
     *
     * @exception IllegalArgumentException occurs if BLANK is passed or if the
     * row or column entered is greater than the size of the board.
     *
     * @exception IllegalMoveException occurs if an already occupied square is
     * mistakenly chosen.
     *
     */
   private boolean play(char row, int col, Symbol currentPlayer) {    
        boolean flag = false;
        try {
            board.update(row, col, currentPlayer);
            flag = true;
        } catch (IllegalMoveException | IllegalArgumentException ex) {
            System.err.println(ex.getMessage());
        }
        return flag;
    }

    /**
     *
     * checkGameStatus() determines if the game has been won or if it has 
     * ended in a tie or if it's ongoing.
     *
     * @param Symbol symbol of the player that made the last move.
     * @param row the row of the last played turn, eg. 'a','b','c' and so on.
     * @param col the column of the last played turn, eg. 1,2,3 and so on.
     *
     * @return GameStatus
     */
   private GameStatus checkGameStatus(Symbol s, char row, int col) {
        GameStatus currentGameStatus = GameStatus.ONGOING;
        if (board.isSameInRow(s, row) || board.isSameInColumn(s, col)
                || board.isSameInBend(s, row, col)
                || board.isSameInBendSinister(s, row, col)) {
            currentGameStatus = GameStatus.WIN;
        } 
        else if (board.getSquareCount() == board.getSize() * board.getSize()) 
        {
            currentGameStatus = GameStatus.TIE;
        } 
        return currentGameStatus;
    }
    
    /**
     *
     * getCurrentPlayer() returns the symbol/player that has to take the next
     * turn.
     *
     * @return Symbol
     */
    private Symbol getCurrentPlayer() {
        return (board.getSquareCount() % 2 == 0) ? Symbol.O
                : Symbol.X;
    }

    
    public static void main(String[] args) {
        Game game = new Game();
        game.start();
    }

    
}

[Campbell Ritchie]

Looking better

Prasanna Raman wrote: . . .

package game;

import java.util.Scanner;

public class MyUtilities {

    private static final Scanner INPUT_SCANNER = new Scanner(System.in);
//...

    public static String getLine() {
        String line="";
        while (line.trim().isEmpty()) {
            line = INPUT_SCANNER.nextLine();
        }
        return line;
    }
}

. . .

You can simplify that method. I think you have put that class in the wrong package.

[Campbell Ritchie]

Prasanna Raman wrote: . . .

package game;

public class Game {

    private final Player[] players;
    private static final int MAX_PLAYERS = 2;
    private static final int MAX_BOARD_SIZE = 9;
    private GameStatus gameStatus;
    private Board board;

 //...
    public void start() {
        int boardSize = -1;
        
        while((boardSize < 3 && boardSize != 0) || boardSize > MAX_BOARD_SIZE) 
        {
            System.out.println("Please enter a board size less than "
                + MAX_BOARD_SIZE + " or press 0 to use the "
                + "default size of 3");
            boardSize = MyUtilities.getIntFromScanner(""
                    + "Please enter an integer between than 3 and " 
                    + MAX_BOARD_SIZE + " or enter 0.");
        }
        board = (boardSize == 0) ? new Board() : new Board(boardSize);
        run(); 
    }
    
//...
    private void run() {
        while (gameStatus != GameStatus.WIN && gameStatus != GameStatus.TIE) {  // !!!
            board.displayState();
            Symbol currentPlayer = getCurrentPlayer();
            System.out.println(currentPlayer + "'s turn. "
                    + "Please enter a location, for eg. a1 for top left");
            String location = MyUtilities.getLine();
 
            while(!location.substring(1).matches(
                    "\\d{1,}")){
                System.err.println("Please enter a valid location");
                location = MyUtilities.getLine();
            }
            char row = location.charAt(0);
            int col = Integer.parseInt(location.substring(1));
            if (play(row, col, currentPlayer)) {
                gameStatus = checkGameStatus(currentPlayer, row, col);
                if (gameStatus == GameStatus.WIN) {
                    board.displayState();
                    System.out.println("Congratulations!! " + 
                            currentPlayer + " "
                            + "wins!!");
                } else if (gameStatus == GameStatus.TIE) {
                    board.displayState();
                    System.out.println("It's a tie!!");
                }
            }
        }
    }
 // ...
}

There is something untidy about the way you have set up the board.
Pleased to see you have got a while for continuing the game (), but you can simplify its condition a lot.
I do not like the regular expression for verifying the entry. If you are going to verify the entry with a regular expression, do so before you try splitting it.

[Prasanna Raman]

Campbell Ritchie wrote:Looking better

Thank you

Campbell Ritchie wrote:You can simplify that method. I think you have put that class in the wrong package.

Sorry, yes. I'll move this class to some package called utility. In what way can I simplify this method?

[Campbell Ritchie]

You can get rid of the initial assignment. If the line entered contains “real” input, there is no need to enter the loop. I posted what I thought was a simpler version about a week ago.

And you were right to put that class in the utility package. Remember you will be using it again and again.

[Prasanna Raman]

Campbell Ritchie wrote:There is something untidy about the way you have set up the board.

Are you referring to the while loop being untidy? Or, can I clean up somewhere else too?

Campbell Ritchie wrote:Pleased to see you have got a while for continuing the game (), but you can simplify its condition a lot.

I can't believe how I didn't use a while loop earlier! Terrible mistake Thank you for pointing me in the right direction.

Campbell Ritchie wrote:I do not like the regular expression for verifying the entry. If you are going to verify the entry with a regular expression, do so before you try splitting it.

OK, I'll use regex on the whole string before splitting. Would you have also used regex? I was thinking about this for a long time before settling on regex. Is there something else that can be done?

[Campbell Ritchie]

I probably wouldn't have used a regex there. The way you wrote it, you are confined to a single‑digit number, and restricted from changing the largest permissible square to 10×10.

[Prasanna Raman]

Yes Is there something else I can try?

[Campbell Ritchie]

You can put a regex on the whole input: one letter, up to two digits, and the square must not be > 26.

[Prasanna Raman]

Thank you. For the play() while loop, is this better?

while(gameStatus == GameStatus.ONGOING) { ... }

I set the gameStatus field to ONGOING when I declare it initially.

[Campbell Ritchie]

A lot better

[Prasanna Raman]

Thank you

Campbell Ritchie wrote:There is something untidy about the way you have set up the board.

Sorry, I don't understand what you're referring to here Could you please clarify?

[Campbell Ritchie]

Asking for 0 to get 3. Maybe System.out.print("Do you want to change the board size from default = 3?: yes/no");
Then you can ask for the size, remembering 1×1 and 2×2 are impossible sizes.

[Prasanna Raman]

Campbell Ritchie wrote:You can get rid of the initial assignment. If the line entered contains “real” input, there is no need to enter the loop. I posted what I thought was a simpler version about a week ago.

Is this the one you're referring to?

public static String getLine(String message)  
{  
   System.out.print(message);  
   String line;  
   while ((line = inScan.nextLine()).trim().isEmpty())  
   {  
      line = inScan.nextLine();  
   }  
   return line;  
}  

I tried to use this one as it is, but since you say "System.out.print(message)", the user types the input immediately after the message but the first line in the while loop assigns "line" to the next line, which will be empty. So I am having to enter the location twice to make it work. That's why I tweaked it a little bit. Am I missing something?

[Campbell Ritchie]

I think you are right. You are going to get alternate lines like that. Sorry, I was wrong there.

[Prasanna Raman]

Here are my updated methods, please take a look.

/**
     *
     * start() gets the board size from the users, initialises the board with
     * the specified size.
     */
    public void start() {
        int boardSize = -1;
        String userPref;
        System.out.println("Do you want to use the default board size of 3? "
                + "Enter Y / N");
        userPref = MyUtilities.getLine();
        while(!userPref.equalsIgnoreCase("Y") && !userPref.
                equalsIgnoreCase("N")) {
            System.err.println("You entered "+userPref+". "
                    + "Please enter only Y or N");
            userPref = MyUtilities.getLine();
        }
        while ((boardSize < 3 || boardSize > MAX_BOARD_SIZE) && 
                userPref.equalsIgnoreCase("N")) {
            System.out.println("Please enter a board size less than "
                    + MAX_BOARD_SIZE);
            boardSize = MyUtilities.getIntFromScanner("Please enter an "
                    + "integer between than 3 and " + MAX_BOARD_SIZE);
        }
        board = (userPref.equalsIgnoreCase("Y")) ? new Board() : new Board(
                boardSize);
        run();
    }
    
    /**
     *
     * run() displays the board state before every turn, determines turns, gets
     * the input from the player and calls play(). After play() returns, checks
     * if the game has been won or if it has ended in a tie or if it has to
     * continue. If there's a result, the game is ended and the result is
     * displayed. If not, the game is continued.
     *
     *
     */
    private void run() {
        while (gameStatus == GameStatus.ONGOING) {
            board.displayState();
            Symbol currentPlayer = getCurrentPlayer();
            System.out.println(currentPlayer + "'s turn. "
                    + "Please enter a location, for eg. a1 for top left");
            String location = MyUtilities.getLine();
 
            while(!location.matches("[a-z][1-9][0-9]*")) {
                System.err.println("You entered "+location+". Please "
                        + "enter a valid location, for eg. a1 for top left");
                location = MyUtilities.getLine();
            }
            char row = location.charAt(0);
            int col = Integer.parseInt(location.substring(1));
            if (play(row, col, currentPlayer)) {
                gameStatus = checkGameStatus(currentPlayer, row, col);
                if (gameStatus == GameStatus.WIN) {
                    board.displayState();
                    System.out.println("Congratulations!! " + 
                            currentPlayer + " "
                            + " wins!!");
                } else if (gameStatus == GameStatus.TIE) {
                    board.displayState();
                    System.out.println("It's a tie!!");
                }
            }
        }
    }

[Campbell Ritchie]

Better; does the output appear nice and tidy to the user?
I do not think your regex will work, however.