Tic-Tac-Toe in Java with GUI

Question

I've wrote a little program to play Tic-Tac-Toe:

public class Control {
    public static void main(String args[]) {
        Gui gui = new Gui();

    }
}

The following classed is used for applying the rules of tic-tac-toe:

import javax.swing.*;

public class TicTacToe {
    public static int count = 0;
    public static String[][] board = new String[3][3];
    public boolean test = true;

    public void buttonClicked(JButton button) {
        if(test) {
            if(button.getText().equals("")) {
                count++;
                if(count % 2 == 1) {
                    button.setText("X");
                }
                if(count % 2 == 0) {
                    button.setText("O");
                }
            }
        }

    }

    public void gameRules(JButton button) {
        if(test) {

            //"X" or "O"?
            String string = button.getText();

            //Gives coordinates of the button
            int x = Character.getNumericValue(button.getName().charAt(0));
            int y = Character.getNumericValue(button.getName().charAt(1));
            board[x][y] = string;

            if(board[0][0] != null && board[0][0].equals(board[1][1]) && board[1][1].equals(board[2][2])) {
                JOptionPane.showMessageDialog(null,string + " won.");
                test = false;
            }

            else if(board[0][2] != null && board[0][2].equals(board[1][1]) && board[1][1].equals(board[2][0])) {
                JOptionPane.showMessageDialog(null,string + " won.");
                test = false;
            }
            else if(count == 9) {
                JOptionPane.showMessageDialog(null, "draw.");
                test = false;
            }
            else {
                for (int i = 0; i < 3; i++) {
                    if (board[i][0] != null && board[i][0].equals(board[i][1]) && board[i][0].equals(board[i][2])) {
                        JOptionPane.showMessageDialog(null, string + " won.");
                        test = false;
                        break;
                    }
                    if (board[0][i] != null && board[0][i].equals(board[1][i]) && board[0][i].equals(board[2][i])) {
                        JOptionPane.showMessageDialog(null, string + " won.");
                        test = false;
                        break;
                    }
                }
            }
        }
    }
}

And finally the class for the GUI:

import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

public class Gui {
    public Gui() {
        JFrame frame = new JFrame();
        JPanel panel = new JPanel();
        TicTacToe ticTacToe = new TicTacToe();
        panel.setLayout(new java.awt.GridLayout(3, 3));
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        for (int i = 0; i < 3; i++){
            for(int j = 0; j < 3; j++) {
                final JButton button = new JButton();
                String string = i +  "" + j;
                button.setText("");
                button.setName(string);
                button.addActionListener(
                        new ActionListener() {
                            public void actionPerformed(ActionEvent e) {
                                ticTacToe.buttonClicked(button);
                                ticTacToe.gameRules(button);
                            }
                        });
                button.setFont(new Font("Arial", Font.PLAIN, 40));
                button.setBorder(BorderFactory.createLineBorder(Color.BLACK));
                panel.add(button);
            }

        }

        frame.add(panel);
        frame.setSize(400,400);
        frame.setVisible(true);




    }
}

I would appreciate any suggestions to improve the code.

Answer 1

Event Dispatching Thread

Since Swing is not thread safe, all Swing UI component creation and modification should be done on the Event Dispatching Thread (EDT). It isn’t a problem with this program, but you could run into it if you also created (say) a javax.swing.Timer in your initiation.

It is an easy change to create the GUI on the EDT. Instead of Gui gui = new Gui(); use:

SwingUtilities.invokeLater(Gui::new);

But I Won???

Play a game, with these moves:

 O | X | O
---+---+---
 X |   | X
---+---+---
 O | X | O

And then put the final X in the center. You win in two directions at once, but because count == 9, the game is a draw?

Separate GUI / Logic

It looks like you tried to separate the GUI from the game logic, but:

• TicTacToe still accesses/manipulates JButton objects
• TicTacToe shows JOptionPane dialogs

So, it is not separate from the Swing GUI; you could not reuse it in an SWT or JavaFX application.

The TicTacToe class should have functions something like:

• bool isValidMove(int x, int y)
• void makeMove(int x, int y)
• bool isGameOver()
• Player getWinner()

and the GUI should convert buttons into x, y locations, display messages, etc

static

Can you play a second game? No. There is no way to reset count for a new game. It would continue to count above 9 with additional moves!

Can you play two games at once? No! There is only one board object. Two simultaneous games would corrupt each other’s board and count variables!

Why are these variables static? It prevents multiple TicTacToe objects from being created. If you removed static from the variables, you could start a new game by creating a new TicTacToe object (and resetting the GUI).

Advanced: Also, you could allow the computer to experiment with different moves, and look several moves in the future, ... but only if you could create these extra TicTacToe boards which aren’t the ones being displayed in the UI.

Simplify Logic

            if(count % 2 == 1) {
                button.setText("X");
            }
            if(count % 2 == 0) {
                button.setText("O");
            }

If count % 2 is not 1, it must be 0. Use an else clause:

            if(count % 2 == 1) {
                button.setText("X");
            } else {
                button.setText("O");
            }

---

This test is very verbose:

        if(board[0][0] != null && board[0][0].equals(board[1][1]) && board[1][1].equals(board[2][2])) {

Who can win on any turn? The player that just made a move, of course. And their symbol is stored in string (a terrible variable name, by the way).

So if the above test was to pass, all the symbols would equal string, so you could write it a little more concisely as:

        if(string.equals(board[0][0]) && string.equals(board[1][1]) && string.equals(board[2][2]) {

If you defined constants (or better, used enum if you are familiar with them):

    public final static String PLAYER_X = "X";
    public final static String PLAYER_O = "O";

and explicitly set the contents of board[][] to either one of those objects, then you could safely test for object identity, instead of equality:

        if(board[0][0] == player && board[1][1] == player && board[2][2] == player) {