TicTacToe GUI needs optimizing

I've been working on a TicTacToe GUI. While the game works with a proper restart, exit and win conditions; I feel that it could still be better optimized.

I'm not particularly concerned with the cosmetics of the GUI. I'm aware of how to make an interface "pretty", so to speak. I would just like to know if there was a better way I could maybe check win conditions or set things up in general.

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

public class BasicGUI {
private static String piece="O";
protected static Boolean player=true;
private static final JFrame frame = new JFrame("Tic Tac Toe");
private static final JPanel panel=new JPanel(new GridLayout(4,3));
protected static final JButton[] cells= new JButton[9];
private static final JButton exitButton=new JButton("Exit");
private static final JButton restartButton=new JButton("Restart");
protected static final JLabel winLabel = new JLabel("Make a move");

public static void main(String[] args){
createWindow();
createButtons();
}

//Set up frame
private static void createWindow(){
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
frame.setSize(450, 600);
frame.setVisible(true);
}

//Add action listeners to buttons
private static void createButtons(){
for(int i=0; i<9; i++){
cells[i]=new JButton();
cells[i].addActionListener(new ButtonHandler());
panel.add(cells[i]);
}
exitButton.addActionListener(new ExitHandler());
restartButton.addActionListener(new RestartHandler());
panel.add(exitButton);
panel.add(restartButton);
panel.add(winLabel);
frame.add(panel);
frame.setVisible(true);
//frame.pack();
}

protected static String getPiece(){
return piece;
}
protected static void setPiece(String s){
piec

A few points:

First of all, your formatting is inconsistent. You have stuff like:

protected static Boolean player=true;

And:

private static final JFrame frame = new JFrame("Tic Tac Toe");

Choose one and stick with it. I suggest the second option, as it is easier to read.

Your checkWinner() method looks nice, but if statements without braces makes me nervous. Try:

protected static Boolean checkWinner() {
    if (checkCells(cells[0], cells[4], cells[8])) { // Diagonal
        return true;
    } else if (checkCells(cells[2], cells[4], cells[6])) {
        return true;
    } else if (checkCells(cells[2], cells[5], cells[8])) { // Horizontal
        return true;
    } else if (checkCells(cells[1], cells[4], cells[7])) {
        return true;
    } else if (checkCells(cells[0], cells[3], cells[6])) {
        return true;
    } else if (checkCells(cells[0], cells[1], cells[2])) { // Vertical
        return true;
    } else if (checkCells(cells[3], cells[4], cells[5])) {
        return true;
    } else if (checkCells(cells[6], cells[7], cells[8])) {
        return true;
    }
    return false;
}

But that now wastes space. I would group everything together:

protected static Boolean checkWinner() {
    return checkCells(cells[0], cells[4], cells[8]) // Diagonal
            || checkCells(cells[2], cells[4], cells[6])
            || checkCells(cells[2], cells[5], cells[8]) // Vertical
            || checkCells(cells[1], cells[4], cells[7])
            || checkCells(cells[0], cells[3], cells[6])
            || checkCells(cells[0], cells[1], cells[2]) // Horizontal
            || checkCells(cells[3], cells[4], cells[5])
            || checkCells(cells[6], cells[7], cells[8]);
}

That looks better, and it makes me less nervous.

Probably a big problem is that your code does not handle ties. I will leave that to you, as it is pretty easy.

StackExchange Code Review Q#80275, answer score: 6