1
\$\begingroup\$

I just created a simple game. Now I'm thinking what could be done better here. I want to get some tips how to make the code better/easier or what to add to it.

Code:

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

public class Test extends JFrame implements ActionListener {
    private JButton[] button = new JButton[9];
    private boolean changePlayer = true;

    private Test() {

        setTitle("Tic Tac Toe");
        setSize(316, 339);
        setLocationRelativeTo(null);
        setLayout(null);
        setResizable(false);

        for (int i = 0; i <= 8; i++) {
            button[i] = new JButton(String.valueOf(i));
            button[i].setBackground(Color.WHITE);
            button[i].setFont(new Font("Arial", Font.BOLD, 0));
            button[i].setFocusPainted(false);
            button[i].addActionListener(this);
            add(button[i]);

        }
        button[0].setBounds(0, 0, 100, 100);
        button[1].setBounds(100, 0, 100, 100);
        button[2].setBounds(200, 0, 100, 100);
        button[3].setBounds(0, 100, 100, 100);
        button[4].setBounds(100, 100, 100, 100);
        button[5].setBounds(200, 100, 100, 100);
        button[6].setBounds(0, 200, 100, 100);
        button[7].setBounds(100, 200, 100, 100);
        button[8].setBounds(200, 200, 100, 100);
    }

    private void restartGame() {

        int playAgain = JOptionPane.showConfirmDialog(null, "Wanna play again?", "You won!", JOptionPane.YES_NO_OPTION);

        if (playAgain == JOptionPane.NO_OPTION) {
            System.exit(0);
        } else {
            for (int i = 0; i <= 8; i++) {
                button[i].setText(String.valueOf(i));
                button[i].setFont(new Font("Arial", Font.BOLD, 0));
                button[i].setForeground(Color.black);
            }


        }
    }

    private void restartGameNoWinner() {

        int playAgain = JOptionPane.showConfirmDialog(null, "Wanna play again?", "Draw!", JOptionPane.YES_NO_OPTION);

        if (playAgain == JOptionPane.NO_OPTION) {
            System.exit(0);
        } else {
            for (int i = 0; i <= 8; i++) {
                button[i].setText(String.valueOf(i));
                button[i].setFont(new Font("Arial", Font.BOLD, 0));
            }
        }
    }

    @Override
    public void actionPerformed(ActionEvent e) {
        for (int i = 0; i <= 8; i++) {
            if (e.getSource() == button[i]) {
                if (changePlayer) {
                    button[i].setText("X");
                    button[i].setFont(new Font("Arial", Font.BOLD, 70));
                    changePlayer = false;
                } else {
                    button[i].setText("O");
                    button[i].setFont(new Font("Arial", Font.BOLD, 70));
                    changePlayer = true;
                }


            }
        }
        if (button[0].getText().equals(button[1].getText()) && button[1].getText().equals(button[2].getText())) {
            button[0].setForeground(Color.RED);
            button[1].setForeground(Color.RED);
            button[2].setForeground(Color.RED);
        } else if (button[0].getText().equals(button[3].getText()) && button[3].getText().equals(button[6].getText())) {
            button[0].setForeground(Color.RED);
            button[3].setForeground(Color.RED);
            button[6].setForeground(Color.RED);
        } else if (button[0].getText().equals(button[4].getText()) && button[4].getText().equals(button[8].getText())) {
            button[0].setForeground(Color.RED);
            button[4].setForeground(Color.RED);
            button[8].setForeground(Color.RED);
        } else if (button[1].getText().equals(button[4].getText()) && button[4].getText().equals(button[7].getText())) {
            button[1].setForeground(Color.RED);
            button[4].setForeground(Color.RED);
            button[7].setForeground(Color.RED);
        } else if (button[2].getText().equals(button[5].getText()) && button[5].getText().equals(button[8].getText())) {
            button[2].setForeground(Color.RED);
            button[5].setForeground(Color.RED);
            button[8].setForeground(Color.RED);
        } else if (button[3].getText().equals(button[4].getText()) && button[4].getText().equals(button[5].getText())) {
            button[3].setForeground(Color.RED);
            button[4].setForeground(Color.RED);
            button[5].setForeground(Color.RED);
        } else if (button[6].getText().equals(button[7].getText()) && button[7].getText().equals(button[8].getText())) {
            button[6].setForeground(Color.RED);
            button[7].setForeground(Color.RED);
            button[8].setForeground(Color.RED);
        }
        if (button[6].getText().equals(button[4].getText()) && button[4].getText().equals(button[2].getText())) {
            button[6].setForeground(Color.RED);
            button[4].setForeground(Color.RED);
            button[2].setForeground(Color.RED);

        }

        if ((button[0].getForeground() == Color.RED && button[1].getForeground() == Color.RED && button[2].getForeground() == Color.RED)
                || (button[0].getForeground() == Color.RED && button[3].getForeground() == Color.RED && button[6].getForeground() == Color.RED)
                || (button[0].getForeground() == Color.RED && button[4].getForeground() == Color.RED && button[8].getForeground() == Color.RED)
                || (button[1].getForeground() == Color.RED && button[4].getForeground() == Color.RED && button[7].getForeground() == Color.RED)
                || (button[2].getForeground() == Color.RED && button[5].getForeground() == Color.RED && button[8].getForeground() == Color.RED)
                || (button[3].getForeground() == Color.RED && button[4].getForeground() == Color.RED && button[5].getForeground() == Color.RED)
                || (button[6].getForeground() == Color.RED && button[7].getForeground() == Color.RED && button[8].getForeground() == Color.RED)
                || (button[2].getForeground() == Color.RED && button[4].getForeground() == Color.RED && button[6].getForeground() == Color.RED)) {
            restartGame();
        } else if ((button[0].getText().equals("X") || button[0].getText().equals("O"))
                && (button[1].getText().equals("X") || button[1].getText().equals("O"))
                && (button[2].getText().equals("X") || button[2].getText().equals("O"))
                && (button[3].getText().equals("X") || button[3].getText().equals("O"))
                && (button[4].getText().equals("X") || button[4].getText().equals("O"))
                && (button[5].getText().equals("X") || button[5].getText().equals("O"))
                && (button[6].getText().equals("X") || button[6].getText().equals("O"))
                && (button[7].getText().equals("X") || button[7].getText().equals("O"))
                && (button[8].getText().equals("X") || button[8].getText().equals("O"))) {
            restartGameNoWinner();
        }
    }

    public static void main(String[] args) {
        Test app = new Test();
        app.setDefaultCloseOperation(EXIT_ON_CLOSE);
        app.setVisible(true);
    }

}
\$\endgroup\$
  • 2
    \$\begingroup\$ See MVC . \$\endgroup\$ – Gnik Jan 2 at 15:25
  • \$\begingroup\$ While short, @Gnik's comment is, imho, the best answer to this question :) \$\endgroup\$ – RobAu Jan 3 at 10:35
1
\$\begingroup\$
    if (button[0].getText().equals(button[1].getText()) && button[1].getText().equals(button[2].getText())) {
        button[0].setForeground(Color.RED);
        button[1].setForeground(Color.RED);
        button[2].setForeground(Color.RED);
    } else if (button[0].getText().equals(button[3].getText()) && button[3].getText().equals(button[6].getText())) {
        button[0].setForeground(Color.RED);
        button[3].setForeground(Color.RED);
        button[6].setForeground(Color.RED);
    } else if (...

This part can be simplified to

// Winning Combinations
Boolean[][] winCombs = {
    {0, 1, 2},
    {0, 3, 6},
    ...
};

for (int i = 0; i < winCombs.length; i++) {
    if (button[winCombs[i][0]].getText().equals(button[winCombs[i][1]].getText()) &&
        button[winCombs[i][1]].getText().equals(button[winCombs[i][2]].getText())) {
        for (int j = 0; j < 3; j++)
            button[winCombs[i][j]].setForeground(Color.RED);
    }
}

And the same for the next part. Any huge block of if conditions can always be simplified to an array and a loop.

\$\endgroup\$
0
\$\begingroup\$

You have a lot of code that do similar things for your 9 buttons. Maybe a custom Button class is a good idea. Also I would merge the two restartGame methods if I was you.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.