4
\$\begingroup\$

This is a Tic Tac Toe game in Java in preparation for my seconds personal project (chess game). I'm having a little problem though and am thinking that my for loop to create those buttons is a mistake. I believe I should instantiate all 9 JButtons implicitly instead of using a for loop because then I know for sure I will be able to compare each and every button. What are your thoughts?

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

/**
 * Created by Usman on 5/17/2016.
 */
public class TicTacToeGUI extends JPanel {

    boolean turn = true;

    public TicTacToeGUI() {
        setLayout(new GridLayout(3,3));

        for(int i=0 ; i<9 ; i++) {
            JButton btn = new JButton("");
            btn.setPreferredSize(new Dimension(70,60));
            btn.setFont(new Font("Arial", Font.PLAIN, 40));
            btn.addActionListener(new ActionListener() {
                @Override
                public void actionPerformed(ActionEvent e) {
                    if(btn.isEnabled()){
                        if(turn){
                            btn.setText("X");
                            turn = !turn;
                        }
                        else{
                            btn.setText("O");
                            turn = true;
                        }
                        btn.setEnabled(false);
                    }
                }
            });
            add(btn);
        }
    }

    public static void main(String []args){
        TicTacToeGUI game = new TicTacToeGUI();
        JFrame frame = new JFrame("Tic Tac Toe");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.getContentPane().add(game);
        frame.pack();
        frame.setVisible(true);
    }
}
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Good job on your first quesiton. \$\endgroup\$ – SirPython May 18 '16 at 19:27
2
\$\begingroup\$

Using a loop isn't inherently wrong, the problem you're anticipating though is right -- you won't have a way of comparing and referencing the buttons since they all become anonymous with your current approach.

Instead, use an array of JButton and simply use the iterator of the loop to instantiate at that location which you can later utilize as an index to reference a specific JButton.

JButton[0] board = new JButton[9];
    for (int i = 0; i < board.length; i++) {
        board[i] = new JButton("");
    }

On the topic of anonymity, as of Java 8 rather than using anonymous inner classes for Action Events you can use Lambda Expressions.

So, you can replace this chunk of code:

 btn.addActionListener(new ActionListener() {
                @Override
                public void actionPerformed(ActionEvent e) {

With:

btn.addActionListener(e -> {

Of course, you close the bracket. This also has the benefit of no longer requiring you to import ActionEvent and ActionListener directly.

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

Think about making your variable names indicate what it is they represent. Instead of having turn, how about calling it something like playXNext.

Try to be consistent with the way you set the variable. Either do this:

if(playXNext){
    btn.setText("X");
} else {                        
    btn.setText("O");
}                
playXNext = !playXNext;

Or do this:

if(playXNext){
    btn.setText("X");
    playXNext = false;
} else {                        
    btn.setText("O");
    playXNext = true;
}                

Whilst both approaches work, if you mix them, for the same variable close together, it just makes the code a little bit harder to read.

\$\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.