patternjavaMinor

Java MVC TicTacToe - follow-up

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

tictactoefollowmvcjava

Problem

Here is my previous question:

Java Tic-Tac-Toe game (implemented through MVC)

I'm reposting again because I made a lot of changes to the code. I wasn't able to implement everything as I don't know some of the stuff, but I tried to do as much as possible.

Summary of changes:

View doesn't have as much logic now.

Model has its own 'logic' for containing the game status separate from the GUI.

Added comments.

Is my MVC better now? What can I do to improve my design, code, and/or style?

View.java

```
import java.awt.GridLayout;
import java.awt.Container;
import java.awt.Dimension;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JButton;
import javax.swing.JOptionPane;

class View extends JFrame {
GridLayout grid = new GridLayout(3, 3); // default grid-size for tic-tac-toe
JButton[] buttons; // an array of the buttons (9 of them)

/**
* Overloaded constructor.
*/
public View() {
super("tic-tac-toe");
setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
addComponentsToPane(getContentPane());
pack();
setVisible(true);

getRootPane().setDefaultButton(buttons[4]);
buttons[4].requestFocus();
}

/**
* Adds the panel along with its buttons to the pane.
*/
public void addComponentsToPane(final Container pane) {
final JPanel panel = new JPanel();
panel.setLayout(grid);
panel.setPreferredSize(new Dimension(300, 300));

buttons = new JButton[9];

for (int i = 0; i < buttons.length; i++) {
buttons[i] = new JButton();
buttons[i].getPreferredSize();
panel.add(buttons[i]);
}

pane.add(panel);
}

/**
* Informs the user who won.
*/
public void informWin(Symbol userSymbol) {
for (int i = 0; i < buttons.length; i++) {
buttons[i].setEnabled(false);
}
JOptionPane.showMe

Solution

So I have some recommendations:

Visibility

It's a little bit confusing the visibility in your classes, all the classes are in the same package?

If I had to do the game (I never code a game, I have mostly experience with webapps) using MVC I would create 3 packages, besides the one of the root project, so I wouls have somethincom.kimae.tictactoe, com.kimae.tictactoe.view, com.kimae.tictactoe.controller and com.kimae.tictactoe.model, and maybe some other packages like helpers, utils and etc...

Since I have this separation by package I would create all my classes that will be used in the context of my general application public, so I would have:

public class Field{}
public class Game{}

and so on.

Now that we have all my classes public I would change the visibility of the attributes to private (I would make exception to attributes that are final....but i don't recommend).

Enum

I don't know if you have some special motive to use the enum like that, but you could just use the names like instead of using userSymbol = Symbol.values()[2]; you could use userSymbol = Symbol.NONE, and in the toString of the field you could do:

@Override
public String toString() {
    return owner.toString();
}

I would put the ENum inside Field, since it concerns a state of Field, and instead initialize the Symbol.NONE in the constructor

Initialization

I would create static factories Field.getDefault()

Responsabilities

So I really thing the Game class is to big, and have to many responsabilities, it is almost acting as a controller.

I would create a class called board which have the matrix for the game and can change the state of the board. Now with this new class the game class would dictate the rules for the game :).

Show me the code!

Here is a draft of your classes

Field

public class Field {
    private Symbol owner;

    private Field(Symbol sym) {
        owner = sym;
    }

    public static Field getDefault(){
        return new Field(Symbol.NONE);
    }

    public Symbol getOwner() {
        return owner;
    }

    public void setOwner(Symbol owner) {
        this.owner = owner;
    }

    @Override
    public String toString() {
        return owner.toString();
    }

    public static enum Symbol {
        X, O, NONE
    }
}

Game

```
/**
* The logic of the game.
*/
public class Game {
private int turnsCounter; // the number of turns since the start of the game
private Symbol userSymbol; // the current Symbol of the player
private boolean didSomeoneWin; // to check if a player won or if it was a tie
private Board board;

/**
* Default constructor.
*
* Initializes the gameGrid array with 9 Field objects and the other
* to their appropriate values.
*/
public Game() {
turnsCounter = 0;
userSymbol = Symbol.NONE;
didSomeoneWin = false;
board = new Board();

}

/**
* Sets the symbol of a specific field.
*
* @param x the value for the x coordinate of the Field
* @param y the value for the y coordinate of the Field
*/
public void setFieldOwner(Symbol owner, int x, int y) {
board.setFieldOwner(userSymbol, x, y);
}

/**
* Returns the owner of a specific field.
* Note: I'm using this just for testing.
*
* @param x the value for the x coordinate of the Field
* @param y the value for the y coordinate of the Field
*/
public Symbol getFieldOwner(int x, int y) {
return board.getFieldOwner(x,y);
}

/**
* Prints the field.
* Note: I'm using this just for testing.
*/
public void printField() {
System.out.println("---PRINTING FIELD---");
for (int i = 0; i < 3; i++) {
for (int j = 0; j < 3; j++) {
System.out.print(getFieldOwner(i, j) + " ");
}
System.out.println();
}
}

/**
* Evaluates the board to see if the game is over, and then checks if the number of turns is maxed out at 9.
* If both are not true, return false;
*/
public boolean isGameOver() {
int[] scores = board.evaluateBoard();
for (int score : scores) {
if (score == 3 || score == -3) {
didSomeoneWin = true;
System.out.println("didSomeoneWin: " + didSomeoneWin);
return true;
}
}

if (turnsCounter == 9) {
return true;
}

return false;
}

/**
* Increments the number of turns.
*/
public void incrementTurnsCounter() {
turnsCounter++;
}

/**
* Returns the number of turns since the start of the game.
*/
public int getTurnsCounter() {
return turnsCounter;
}

/**
* Sets the user symbol to a Symbol depending on whether the number of turns is even or odd.
*/
public void setUserSymbol() {
if (turnsC

Code Snippets

public class Field{}
public class Game{}

@Override
public String toString() {
    return owner.toString();
}

public class Field {
    private Symbol owner;

    private Field(Symbol sym) {
        owner = sym;
    }

    public static Field getDefault(){
        return new Field(Symbol.NONE);
    }

    public Symbol getOwner() {
        return owner;
    }

    public void setOwner(Symbol owner) {
        this.owner = owner;
    }

    @Override
    public String toString() {
        return owner.toString();
    }

    public static enum Symbol {
        X, O, NONE
    }
}

/**
 * The logic of the game.
 */
public class Game {
    private int turnsCounter; // the number of turns since the start of the game
    private Symbol userSymbol; // the current Symbol of the player
    private boolean didSomeoneWin; // to check if a player won or if it was a tie
    private Board board;

    /**
     * Default constructor.
     * 
     * Initializes the gameGrid array with 9 Field objects and the other
     * to their appropriate values.
     */
    public Game() {
        turnsCounter = 0;
        userSymbol = Symbol.NONE;
        didSomeoneWin = false;
        board = new Board();

    }

    /**
     * Sets the symbol of a specific field.
     * 
     * @param x     the value for the x coordinate of the Field
     * @param y     the value for the y coordinate of the Field
     */
    public void setFieldOwner(Symbol owner, int x, int y) {
        board.setFieldOwner(userSymbol, x, y);
    }

    /**
     * Returns the owner of a specific field.
     * Note: I'm using this just for testing.
     * 
     * @param x     the value for the x coordinate of the Field
     * @param y     the value for the y coordinate of the Field
     */
    public Symbol getFieldOwner(int x, int y) {
        return board.getFieldOwner(x,y);
    }

    /**
     * Prints the field.
     * Note: I'm using this just for testing.
     */
    public void printField() {
        System.out.println("---PRINTING FIELD---");
        for (int i = 0; i < 3; i++) {
            for (int j = 0; j < 3; j++) {
                System.out.print(getFieldOwner(i, j) + " ");
            }
            System.out.println();
        }
    }

    /**
     * Evaluates the board to see if the game is over, and then checks if the number of turns is maxed out at 9.
     * If both are not true, return false;
     */
    public boolean isGameOver() {
        int[] scores = board.evaluateBoard();
        for (int score : scores) {
            if (score == 3 || score == -3) {
                didSomeoneWin = true;
                System.out.println("didSomeoneWin: " + didSomeoneWin);
                return true;
            }
        }

        if (turnsCounter == 9) {
            return true;
        }

        return false;
    }

    /**
     * Increments the number of turns.
     */
    public void incrementTurnsCounter() {
        turnsCounter++;
    }

    /**
     * Returns the number of turns since the start of the game.
     */
    public int getTurnsCounter() {
        return turnsCounter;
    }

    /**
     * Sets the user symbol to a Symbol depending on whether the number of turns is even or odd.
     */
    public void setUserSymbol() {
        if (turnsCounter % 2 == 1) {
            userSymbol = Symbol.O;
        } else {
            userSymbol = Symbol.X;
        }
    }

    /**
     * Returns the current user symbol.
     */
    public Symbol getUserSymbol() {
        return userSymbol;
    }

    /**
     * Returns true if someone has won the game. Otherwise, f

public class Controller implements ActionListener {
    private Game game;
    private View view;

    /**
     * Overloaded constructor. Initializes the game and view, and
     * adds the action listeners to the buttons in view.
     *
     * @param   an instance of the Game class.
     * @param   an instance of the View class.
     */
    public Controller(Game game, View view) {
        this.game = game;
        this.view = view;
        addActionListeners();
    }

    /**
     * Adds an action listener to every button.
     */
    private void addActionListeners() {
        for (int i = 0; i < view.buttons.length; i++) {
            view.buttons[i].addActionListener(this);
        }
    }

    /**
     * Increments the number of moves since the start of the game, and
     * sets the user symbol. It then finds out what x and y coordinates that button
     * corresponds to in the Game object.
     * Examples: button[0] would be Field[0][0]. button [1] would be Field[0][1].
     * button[5] would be Field[1][2].
     * 
     * It then sets the owner of the field in the Game object, and modifies the View buttons.
     *
     * @param e     the action performed. In this game, it would be a mouse click.
     */
    @Override
    public void actionPerformed(ActionEvent e) {
        if (game.isGameOver() == false) {
            game.incrementTurnsCounter();
            game.setUserSymbol();
            int index = getMove((JButton) e.getSource());
            int x = 0; // row coordinate
            int y = 0; // column coordinate
            switch (index) {
                case 0: x = 0;
                        y = 0;
                        break;
                case 1: x = 0;
                        y = 1;
                        break;
                case 2: x = 0;
                        y = 2;
                        break;
                case 3: x = 1;
                        y = 0;
                        break;
                case 4: x = 1;;
                        y = 1;
                        break;
                case 5: x = 1;
                        y = 2;
                        break;
                case 6: x = 2;
                        y = 0;
                        break;
                case 7: x = 2;
                        y = 1;
                        break;
                case 8: x = 2;
                        y = 2;
                        break;
                default: break;
            }
            game.setFieldOwner(game.getUserSymbol(), x, y);
            ((JButton) e.getSource()).setText((game.getUserSymbol()).toString());
            ((JButton) e.getSource()).setEnabled(false);
        }
    }

    /**
     * Returns the index of the current JButton.
     * 
     * @param       the button that was clicked.
     */
    public int getMove(JButton button) {
        int index = 0;
        for (int i = 0; i < 9; i++) {
            if (button == view.buttons[i]) {
                index = i;
            }

Context

StackExchange Code Review Q#125721, answer score: 2

Revisions (0)

No revisions yet.