Tic Tac Toe - Stage 1: console PvP

I'm currently working on a Tic Tac Toe project to improve my programming skills, and I'm simultaneously learning Git.

This is my GitHub page for my Tic Tac Toe project. I will post the code below so you don't have to go to GitHub. But if you could give me some small feedback whether I have pushed/committed things in the correct way, that'd be nice.

Anyway, before I post my code, here is what I have in mind - you might want to consider this when answering (I would also appreciate if you can give some feedback to this approach, and maybe some ideas):


I decided to start a Tic Tac Toe project, and see it through. Here is
what I'm currently thinking of (the various stages may be vague for
now, but that's OK):

• start with a simple console program, for two players

• create an artificial intelligence (AI), which

makes random moves (easy to play against)

• add a game menu which offers

the option of single- and multiplayer

• add a stronger AI using the

Minimax algorithm and adjust the menu accordingly

• make a simple GUI

version

• improve the GUI in various ways (current ideas - no idea how

hard they are to implement: )

• add multiple themes

• add animations for

the moves

• add sound

• switch background color (light/dark)

This would be
my initial goal. I have other ideas, which might come into play as
well, as in:

• keep track of statistics

• make it playable on the web

And here is the code (5 classes in total, as of now):

package game;

public enum Sign {
  X, O;
}

package game;

public class Main {
    public static void main(String[] args) {
        // play Tic Tac Toe
        Game game = new Game();
        game.playGame();
    }
}

```
package game;

public class Player {
private String name;
private Sign sign;

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

public void setSign(Sign sign) {

Sign is good, main is good, Player is okay, I don't like Board, and Game is a mess. Relatively speaking. It's pretty naturally written, but you seem to be lacking some tools. Specifically, the idea of using object references as more than just storage of data, and "tricks" like using arithmetic in accessing data structures.

First, learn to remove duplication.

private void declareWinner() {
    if(isTurnP1)
        System.out.printf("%s (%s) won%n", player1, player1.getSign());
    else
        System.out.printf("%s (%s) won%n", player2, player2.getSign());
}

This ought to just take a player argument.

Then you can do this:

private void declareWinner(Player winner) {
    System.out.printf("%s (%s) won%n", winner, winner.getSign());
}

Second, I think you had some trouble separating the view and the model - you refer to board cells internally as Strings, and the result is having to do things like this:

private void setMark(String move) {
    for(int i=0; i < board.getBoard().length; ++i) {
        for(int j=0; j < board.getBoard()[i].length; ++j) {
            if(board.getBoard()[i][j].equals(move)) {
                board.getBoard()[i][j] = isTurnP1 ? player1.getSign() + "" 
                                                  : player2.getSign() + "";
            } 
        }
    }
}

There's two ways I quickly see about this;
First is just making a list instead of a 2d array.
That way you can parse the input as a number and jump to the position in the board.

The other, involves numbers as well, but you just leave it as a 2d array, and use the modulo % operator instead. It gives you the remainder of a division, so 8 mod 3 is 2. Since you start at 1 instead of 0, you would need to do board.getBoard()[(move-1)/3][(move-1)%3] to get the tile you wanted to get at.

Look up Integer.parseInt. Maybe handle it in the scanner, but beware of format exceptions.

private void callForTurn() {
    System.out.println();
    if(isTurnP1)
        System.out.printf("%s's turn (%s)%n", player1, player1.getSign());
    else
        System.out.printf("%s's turn (%s)%n", player2, player1.getSign());
}

Duplication again, take a player argument to resolve to

private void callForTurn(Player player) {
    System.out.println();
    System.out.printf("%s's turn (%s)%n", player, player.getSign());
}

I think if you were to replace your isTurnP1 variable with a Player variable instead (activePlayer?), a lot of the fixes in Game would become easier to apply.

StackExchange Code Review Q#116545, answer score: 6