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

After seeing Tic Tac Toe in MVC, I attempted to create something similar. My program uses the Swing library and it's relatively simple. When I run the game, it's just a 3x3 grid with no extra features.

Can anyone give me tips on how to improve my overall MVC design / code? I did a lot of reading, but I sure I don't fully understand the concept yet. My view.java file is extremely lengthy.

View.java

```
import java.awt.*;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JButton;

class View extends JFrame {

GridLayout grid = new GridLayout(3, 3);
JButton[] buttons;
Model model;
boolean gameEnd;

public View(Model model) {
super("tic-tac-toe");
setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
addComponentsToPane(getContentPane());
pack();
setVisible(true);
this.model = model;
gameEnd = false;
}

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);
}

public boolean checkBoardForWin() {
int x_sum_first_row, x_sum_second_row, x_sum_third_row,
x_sum_first_column, x_sum_second_column, x_sum_third_column,
x_sumDiagonalLR, x_sumDiagonalRL;

x_sum_first_row = x_sum_second_row = x_sum_third_row
= x_sum_first_column = x_sum_second_column = x_sum_third_column
= x_sumDiagonalLR = x_sumDiagonalRL = 0;

int o_sum_first_row, o_sum_second_row, o_sum_third_row,
o_sum_first_column, o_sum_second_column, o_sum_third_column,
o_sumDiagonalLR, o_sumDiagonalRL;

o_sum_first_row = o_sum_second_row = o_sum_

MVC and your structure


Can anyone give me tips on how to improve my overall MVC design / code? I did a lot of reading, but I sure I don't fully understand the concept yet. Any help is appreciated.

MVC is a bit of a confusing concept, as different people use the term differently. Just looking at the different images at different wikipedia pages shows that MVC can be implemented in different ways.

I don't think that you need to worry about implementing it 100% correct for now, but you should try to follow the basic idea.

So basically, you have three components:

• The Model: Holds the data and may contain business logic

• The View: Displays a representation of the data

• The Controller: Processes user input and applies it to the view and/or model

You then somehow have to string these components together, which is where the approaches to MVC differ a bit.


My view.java file is extremely lengthy..

That is because it doesn't just display the data, it actually contains program logic, which it shouldn't. It also holds the data (the current state of the game), which it also should not do.

Your Model

First of all, you need to document your code with JavaDoc comments for functions and public fields.

It's not really clear what movesCounter does, or what choice setChoice sets (especially without looking at the implementation).

Your model also has a very generic name. What I would have excepted is something like this:

/**
 * One field of the game grid.
 */
class Field {
    private Symbol owner;

    // getters, setters, constructor
}

/**
 * A represenation of an owner of a field.
 */
enum Symbol {
    X, O, NONE
}

class Game {
    private Field[][] gameGrid;

    /** 
     * sets the owner of the field determined by x and y.
     *
     * @param  owner  the new owner of the field
     * @param  x x coordinate
     * @throws IllegalMoveException in case the owner cannot be set
     */
    public setFieldOwner(Symbol owner, int x, int y) throws IllegalMoveException {
        // ...
    } 

    public boolean isGameOver()
    public Symbol getWinner()
    ...
}

Now you have a game field, which contains the current state of the game, as well as the logic of the game.

There is room for improvement here, but it should be good enough to show you the general idea.

Your View

As I said above, your view does too much. It should only show the current state of your model, nothing else.

Generally, it might implement an interface such as this:

interface View {

    // when this method is called, you would change the view of the corresponding element
    public setFieldOwner(Symbol owner, int x, int y);

    // here, you could show a message, and disable buttons, show start new game button, etc
    public informWin(Symbol winner);

    public informStart(Symbol winner);
}

Your Controller

Your controller is fine. If you use a similar approach to the one outlined above, you would need to change it a bit, but generally, it would still perform the same task as now (get input, apply it to model and view).

You should probably add a more generic InputInterface instead of using an ActionListener directly to separate the controller from the concrete GUI. It would probably only have one method: getMove, which should return the desired move in some way (either create a move class, or just use an array).

Misc

• fields should be private to encapsulate the concrete implementation.

• variable names should be camelCase, not snake_case.

• always use curly brackets even for one line statements to avoid future bugs and to increase readability.

• the name setX generally implies a setter, which would accept an argument and apply it to the object. It's a bit confusing that you use it differently.

StackExchange Code Review Q#125248, answer score: 4