Tic-tac-toe Design

[Prasanna Raman]

Hello,

I am trying to learn object oriented programming design. Kindly look at this code and point out design flaws or areas for improvement.

package game.tictactoe;

public class Game {
    String[] gameState = new String[9];
    Player player1;
    Player player2;

// updates the gamestate array with the latest move

    public void updateStatus(int position, String symbol) {
        gameState[position] = symbol;    
    }
    
    public Game() {
        player1 = new Player(1);
        player2 = new Player(2);
        for(int i = 0; i < 9; i++) {
            gameState[i] = "";
        }
    }

// checks if game over. If game over, return winner or tie, else return "Game in prgress"
    
    public String getGameStatus() {
        if((gameState[0].equals(gameState[1]))
           && gameState[0].equals(gameState[2])
           && gameState[1].equals(gameState[2])
           && !gameState[0].equals("")){
            return gameState[0];
        }
        else if((gameState[3].equals(gameState[4]))
           && gameState[3].equals(gameState[5])
           && gameState[4].equals(gameState[5])
           && !gameState[3].equals("")){
            return gameState[3];
        }
        else if((gameState[6].equals(gameState[7]))
           && gameState[6].equals(gameState[8])
           && gameState[7].equals(gameState[8])
           && !gameState[6].equals("")){
            return gameState[6];
        }
        else if((gameState[0].equals(gameState[3]))
           && gameState[0].equals(gameState[6])
           && gameState[3].equals(gameState[6])
           && !gameState[0].equals("")){
            return gameState[0];
        }
        else if((gameState[1].equals(gameState[4]))
           && gameState[1].equals(gameState[7])
           && gameState[4].equals(gameState[7])
           && !gameState[1].equals("")){
            return gameState[1];
        }
        else if((gameState[2].equals(gameState[5]))
           && gameState[2].equals(gameState[8])
           && gameState[5].equals(gameState[8])
           && !gameState[2].equals("")){
            return gameState[2];
        }
        else if((gameState[0].equals(gameState[4]))
           && gameState[0].equals(gameState[8])
           && gameState[4].equals(gameState[8])
           && !gameState[0].equals("")){
            return gameState[0];
        }
        else if((gameState[2].equals(gameState[4]))
           && gameState[2].equals(gameState[6])
           && gameState[4].equals(gameState[6])
           && !gameState[2].equals("")){
            return gameState[2];
        }
        else {
            for(int i=0; i < 9; i++) {
                if(gameState[i].equals("")) {
                    return "Game in progress";
                }
            }
            return "It's a tie!!";
        }
    }

    public String play(int position) {
        if(!player1.hasPlayed()) {
            player1.played(true);
            player2.played(false);
            updateStatus(position,player1.getSymbol());
            return player1.getSymbol();
        }
        else {
            player2.played(true);
            player1.played(false);
            updateStatus(position,player2.getSymbol());
            return player2.getSymbol();
        }
    }
}

package game.tictactoe;

public class Player {
    private boolean hasPlayed;
    private String symbol;
    private int id;
    
    Player(int id) {
        this.id = id;
        if(id == 1) {
            hasPlayed = false;
           symbol = Symbols.X.toString();
        }
        else {
            hasPlayed = true;
            symbol = Symbols.O.toString();
        }
    }
    
    public void played(boolean flag) {
        hasPlayed = flag;
    }
    public boolean hasPlayed() {
         return hasPlayed; 
    }
    public String getSymbol() {
        return symbol;
    }
}

package game.tictactoe;

public enum Symbols {
    X,O;
}

[Campbell Ritchie]

I like the last part, the enum.
I don't like any of the rest of it, I am afraid. It looks like the sort of code you get when you spend half an hour thinking and all day writing. It is supposed to be the other way round, as I told you this morning.
There is something very wrong with setting all the fields in a Player from an ID number. There is something wrong with such a large block of if‑elses.
Let's see your design first. You needn't expect to get decent code until you have got a decent design.

Take the enum. How can you get elements out of an enum in order? If you don't know, try the Java tutorials and the Java Language Specification. That means you can create exactly the same number of Player instances as there are elements in the enum.

That also means you have got about 5% of the problem solved. You still will have to work on the other 95%.

[Winston Gutkowski]

Prasanna Raman wrote:I am trying to learn object oriented programming design.

Prasanna,

Please DontWriteLongLines. It makes your thread very hard to read, and it's actually bad coding practice.
I've broken yours up this time, but for future reference, please remember:
80 characters max.
(the SSCCE page actually recommends 62)
And that includes string literals AND comments AND long method calls and 'if' statements.

Thanks.

Winston

[fred rosenberger]

My primary criticism is:

Not enough comments.

[Prasanna Raman]

Winston Gutkowski wrote:

Prasanna Raman wrote:I am trying to learn object oriented programming design.

Prasanna,

Please DontWriteLongLines. It makes your thread very hard to read, and it's actually bad coding practice.
I've broken yours up this time, but for future reference, please remember:
80 characters max.
(the SSCCE page actually recommends 62)

Thank you, Winston. And sorry. I will keep this in mind.

[Prasanna Raman]

Campbell Ritchie wrote:I like the last part, the enum.
I don't like any of the rest of it, I am afraid. It looks like the sort of code you get when you spend half an hour thinking and all day writing. It is supposed to be the other way round, as I told you this morning.
There is something very wrong with setting all the fields in a Player from an ID number. There is something wrong with such a large block of if‑elses.
Let's see your design first. You needn't expect to get decent code until you have got a decent design.

Take the enum. How can you get elements out of an enum in order? If you don't know, try the Java tutorials and the Java Language Specification. That means you can create exactly the same number of Player instances as there are elements in the enum.

That also means you have got about 5% of the problem solved. You still will have to work on the other 95%.

Thank you, Campbell. I will change the enum part first. Please bear with me, I'll keep working on it until you're happy with my design.

[Campbell Ritchie]

Prasanna Raman wrote: . . . Thank you, Campbell. I will change the enum part first. . . .

The one bit I said I liked, and you want to change that.

[Prasanna Raman]

No, I meant the part where we can get values from enum!!

[Prasanna Raman]

I've now changed the game constructor; it no longer initializes the player objects by receiving an id. Instead, it now creates player objects using the enum and adds them to a Player array.

Here is the code:

public Game() {
        int playerCount = 0;
        players = new Player[Symbols.values().length];
        for (Symbols s : Symbols.values()) { 
            players[playerCount] = new Player(s);
            playerCount++;
        }
        for (int i = 0; i < 9; i++) {
            gameState[i] = "";
        }
    }

Have I done the above right?


Then, when I looked at the code today, it seemed to me that the play() method does not belong in Game(). It looks better if defined in Player.

However, I don't know how it should look. I've made a few updates to play() but still don't know how to make it work in Player.

public String play(int position) {
            this.played(true);
            updateStatus(position, this.getSymbol());
            return this.getSymbol();
        } 
    }

If I write the above code in Player class, should updateStatus() be in this method? Is it the responsibility of the player or the game to update its status? I'd think game, but how and from where do I call it? Should it be the user class that calls play() and then updateStatus()? By user class I mean the below method:

 public void mousePressed(MouseEvent e) {
        JButton button = (JButton) e.getSource();
        button.setText(game.play(Integer.parseInt(button.getName())));
        label.setText(game.getGameStatus());
    }

I am at my wits' end trying to figure this out! Kindly let me know where I am going wrong.

[Campbell Ritchie]

Don't try so many things simultaneously. Try one part and get it working before you even think about coding the second part.
As I mentioned yesterday, what is your design? Tell us that. No code, please.
Your code looks like somebody who is guessing. You can guess 1000000 times and you will probably get a correct working version. You will simply have to inspect the other 999999 versions to exclude them. That is easy.

Or you can do it the hard way. You can design it properly and think about what you are doing. Then you can try 1 version and it will be correct.

The only bit that is even approaching acceptable is the loop where you initialise the players. But why are you trying to combine an index with a for‑each loop? Why not use an ordinary for loop?

[Maneesh Godbole]

I am at my wits' end trying to figure this out! Kindly let me know where I am going wrong.

I would say you are digging your own grave by ignoring the excellent advise given to you in the other thread

Bear wrote:Develop the model of the game. That is, the classes that model the game, it's behavior and state.

Campbell wrote:You should spend all day thinking about the TicTacToe example. Once you have done so, you can write the entire model for a game in half an hour.

Winston wrote:But the main point, as everybody else has said, is to start simple.

So, if you went out to the shop and bought yourself a tic tac toe game, what all would you find in the box?
A board. Noughts and crosses and a rule book.
Now can you identify all the basic building blocks of your game model?

[Campbell Ritchie]

You need two more building blocks: one person to play nought and one person to play cross.

[Campbell Ritchie]

At the risk of being thrown amongst the snakes, I would never think of buying a noughts‑and‑crosses game. I have always used a sheet of paper and a pencil

[Prasanna Raman]

So, just to confirm if my understanding is right, you first want me to list all the classes and just the method stubs inside it without any code?

Thanks,
Prasanna

[Campbell Ritchie]

No, we would like to see your design. Listing classes comes later.

[Prasanna Raman]

Sorry, I am completely lost I thought by design you meant laying out the classes and methods. Forgive my ignorance, please tell me an example of what you mean by design.

[Campbell Ritchie]

It describes how you would play a game. Once you have that description, you can work out which classes you will need.

[Winston Gutkowski]

Prasanna Raman wrote:So, just to confirm if my understanding is right, you first want me to list all the classes and just the method stubs inside it without any code?

That's not a bad way to go, but even better might be to write out the game requirements in detail and in English (or your native language) first.

You might also find the WhatNotHow page (←click) useful to read.

It may also be worth mentioning that the logic for a winning strategy at tic-tac-toe is NOT simple (as you'll soon discover if you try larger board sizes). So if you were thinking about having the computer as one of your "players", I'd put that on hold for the moment and simply start out with a "game" that puts X's and O's where it's told to, and declares a winner when it detects a full line of either.

Winston

[Prasanna Raman]

Here's my description:

There's a board, and there are 2 players. One player will play 'X' and the other will play 'O'.

There is a rule book which will include:

* How to play the game - that is, what constitutes a legal move
* When does the game finish?
* How is the winner determined?

Does this sound right? If yes, please suggest what my next step should be.

Thanks,
Prasanna

[Winston Gutkowski]

Prasanna Raman wrote:Does this sound right? If yes, please suggest what my next step should be.

Write out the "rule book" - all of it, and in detail.

Winston

[Prasanna Raman]

How to play the game - that is, what constitutes a legal move

Each player makes a move by placing his symbol ("X" or "O") on an empty square in the board.

The players take turns - i.e., the player has to wait after making one move for the other player to complete his move.

When does the game finish?
The game finishes when either

Any row/column/diagonal of squares contain the same symbol ---- (A)

All the squares are filled up but condition (1) is not satisfied ------ (B)

How is the winner determined?

If (A) is true, the player whose symbol satisfied the condition is the winner.

If (B) is true, the game is a tie.

[Campbell Ritchie]

That sounds good, but it needs much more detail. Just from your first point: what shape is the board? How many squares has it? How are they arranged?

[Prasanna Raman]

As suggested, I've described the first 2 points in more detail here. I have come up with as much detail as I can about the game. Not sure if I'm missing something.


How to play the game - that is, what constitutes a legal move

Each player makes a move by placing his symbol ("X" or "O") on an empty square in the board. The board is made up of 9 squares (I'm starting with a 3*3 board) resembling a 3*3 matrix

The players take turns - i.e., the player has to wait after making one move for the other player to complete his move. So a player shouldn't be able to make a move after he's played until the other player has made his move

When does the game finish?
The game finishes when either

Any row/column/diagonal of squares contain the same symbol ---- (A)

All the squares are filled up but condition (1) is not satisfied ------ (B)

How is the winner determined?

If (A) is true, the player whose symbol satisfied the condition is the winner.

If (B) is true, the game is a tie.

[Prasanna Raman]

If the above description is right, what should be the next thing I should be doing?

[Campbell Ritchie]

Write the description so everybody can understand it. Most people don't know what a 3*3 matrix is.
Get the sections in the correct order. Start by saying how many people play. Describe the board before you describe the moves.

[Prasanna Raman]

Board
The board is made up of 9 squares divided into 3 rows and 3 columns.

Symbols
There are 2 symbols used to play the game, "X" and "O".

Players
There are 2 human players playing against each other. One player will have his symbol as "X" and the other "O".

Game
Each player makes a move by placing his symbol ("X" or "O") on an empty square in the board.
The players take turns - i.e., the player has to wait after making his move for the other player to complete his move. So a player shouldn't make a move after he's played until the other player has made his move.

The game finishes when either

Any row/column/diagonal of squares contain the same symbol ---- (A)

There are no empty squares on the board but (A) is not satisfied ------ (B)

The winner is determined as follows:

If (A) is true, the player whose symbol satisfied the condition is the winner.

If (B) is true, the game is a tie.

[Campbell Ritchie]

Getting better

Prasanna Raman wrote: . . .
There are 2 human players playing against each other. One player will have his symbol as "X" and the other "O".
. . .

Important point there. It means you are not going to try to program game logic into the app. Makes it much easier.
Work out how you are going to verify conditions A and B. How do you do it when playing noughts‑and‑crosses with a pencil and paper?

[Prasanna Raman]

Getting better

That's heartening

Verifying (A):
We have a winner if

When a symbol is entered in a square, the row that contains the square has the same symbol in all its squares.

Or, the column that contains the square has the same symbol in all its squares.

Or, the diagonal that contains the square has the same symbol in all its squares.

Verifying(B):
It's a tie if

There are no empty squares on the board and we don't have a winner by the rules described in (A)

[Campbell Ritchie]

There is another way to verify each of those conditions. Probably several ways to do it. I challenge you to find another way for each. Also describe very simply how you intend to verify a row/column/diagonal.

[Prasanna Raman]

Campbell Ritchie wrote:There is another way to verify each of those conditions. Probably several ways to do it. I challenge you to find another way for each.

Do you mean in a computer program? If it's on a physical board, I am out of ideas The only way I can think of doing it is what I wrote before - checking all the neighbouring boxes to see if they have the same symbol.

Campbell Ritchie wrote: Also describe very simply how you intend to verify a row/column/diagonal.

Are you asking how I intend to do it inside my program? How am I going to store the board etc. ?

[Campbell Ritchie]

Start with a physical board. When you have some options, you can decide which to code. The more detail you use to describe things at this stage, the easier it will be to translate into code.

[Campbell Ritchie]

Shall I let you out of your misery?
You can check whether the game has been won by checking all three rows, all three columns and both diagonals. You can check whether the game has finished without a winner by counting nine moves.

There are bound to be other ways to do it.

[Prasanna Raman]

Campbell Ritchie wrote:
You can check whether the game has been won by checking all three rows, all three columns and both diagonals. You can check whether the game has finished without a winner by counting nine moves.

Thank you, Campbell! Sorry, I'm going to have to ask you another dumb question! Does this method not require more checks than the one I wrote earlier? In what I wrote above, I was going to check only the neighbouring cells of the square that was populated last.

[Campbell Ritchie]

I shall answer that question when you get to the stage of trying to code it.

[Prasanna Raman]

Thank you. What should I do next? Should I start thinking in terms of classes or not yet?

[Campbell Ritchie]

Design from the centre towards the periphery.
Code from the periphery towards the centre. Give each class a main method which sets off lots of tests. When you put the whole app tog ether either delete the main method or comment it out or give it private access, so it cannot be called.

Yes, I think you are at the stage where you can start thinking of classes. I have spent a really long time thinking about your problem (well over 5″) and looked in a good number of other sources (0) and dscussed it with a nice round number (0) of colleagues and I think you will end up with about 6 classes.

[Prasanna Raman]

I thought about the design and have come up with the following classes:

Game
Attributes - Board and players.
Behaviour - update the board, check if game has ended and/or find a winner and regulate players' turns.

Board
Attributes - rows and columns. So I will need a data structure to store the board in.
Behaviour - get updated by the game to reflect its latest state

Player
Attributes - symbol
Behaviour - play

Symbols - an enum class storing "X" and "O"

Please take a look and let me know the flaws in this design.

[Campbell Ritchie]

No flaws, but I think there are at least two classes missed out.

[Prasanna Raman]

Thank you. The only other thing I can think of at this time is splitting up the board further (into rows and columns or squares/grid?) but I am not to visualize how that is going to make my design better or even help me achieve the correct result Please help.