4
\$\begingroup\$

I'm new to programming and want to check if I'm using best coding practices in my program.

package scott.TicTacToe.main;

import java.awt.GridLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JOptionPane;

public class TicTacToe implements ActionListener{
    private JFrame window = new JFrame("Tic-Tac-Toe");
    private JButton[] buttons = new JButton[9];
    private String letter = "";
    private int count = 0;
    private boolean win = false;

    public TicTacToe(){
        window.setSize(300, 300);
        window.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        window.setLayout(new GridLayout(3, 3));

        for(int i = 0; i < 9; i++){
            buttons[i] = new JButton();
            window.add(buttons[i]);
            buttons[i].addActionListener(this);
        }

        window.setVisible(true);

    }

    public void actionPerformed(ActionEvent event) {

        count++;
        if(count % 2 != 0){
            letter = "X";
        } else {
            letter = "O";
        }

        for(int i = 0; i < 9; i++){
            if (event.getSource() == buttons[i]){
                buttons[i].setText(letter);
                buttons[i].setEnabled(false);
            }

            //Horizontal Wins
            if(buttons[0].getText().equals(buttons[1].getText()) && buttons[1].getText().equals(buttons[2].getText()) && !buttons[0].getText().equals("")){
                win = true;
            } else if (buttons[3].getText().equals(buttons[4].getText()) && buttons[4].getText().equals(buttons[5].getText()) && !buttons[3].getText().equals("")){
                win = true;
            } else if (buttons[6].getText().equals(buttons[7].getText()) && buttons[7].getText().equals(buttons[8].getText()) && !buttons[6].getText().equals("")){
                win = true;
            }

            //Vertical Wins
            else if(buttons[0].getText().equals(buttons[3].getText()) && buttons[3].getText().equals(buttons[6].getText()) && !buttons[0].getText().equals("")){
                win = true;
            } else if(buttons[1].getText().equals(buttons[4].getText()) && buttons[4].getText().equals(buttons[7].getText()) && !buttons[1].getText().equals("")){
                win = true;
            } else if(buttons[2].getText().equals(buttons[5].getText()) && buttons[5].getText().equals(buttons[8].getText()) && !buttons[2].getText().equals("")){
                win = true;
            }

            //Diagonal Wins
            else if(buttons[0].getText().equals(buttons[4].getText()) && buttons[4].getText().equals(buttons[8].getText()) && !buttons[0].getText().equals("")){
                win = true;
            } else if(buttons[6].getText().equals(buttons[4].getText()) && buttons[4].getText().equals(buttons[2].getText()) && !buttons[6].getText().equals("")){
                win = true;
            } else {
                win = false;
            }

            if(win == true){
                JOptionPane.showMessageDialog(null, letter + " wins!");
            } else if(count == 9){
                JOptionPane.showMessageDialog(null, "It's a draw!");
                count = 0;
            }
        }

    }

    public static void main(String[] args) {

        new TicTacToe();

    }

}
\$\endgroup\$
3
  • \$\begingroup\$ Why do you feel the game needs optimizing? \$\endgroup\$
    – Pimgd
    Feb 12, 2015 at 10:57
  • \$\begingroup\$ What version of Java are you using? \$\endgroup\$
    – barq
    Feb 12, 2015 at 11:27
  • \$\begingroup\$ I was using Java 7 Update 67, never realized I didn't update it. Now I have the latest version. \$\endgroup\$ Feb 13, 2015 at 8:53

1 Answer 1

1
\$\begingroup\$

You could perhaps look at separating the win condition by sticking it in another method then call it in ActionPerformed.

Outside of that, you could replace:

if(count % 2 != 0){
        letter = "X";
    } else {
        letter = "O";
    }

with a ternary operator such as:

letter =(count%2!=0)?"X":"O";

That's all I can think off the top of my head.

\$\endgroup\$
2
  • 2
    \$\begingroup\$ No problem! Last thing that I can see is with your getWins() method. Check out my post codereview.stackexchange.com/questions/80275/… In there I have two methods called checkWinner and checkCells. They do a similar thing and it looks a bit neater \$\endgroup\$ Feb 11, 2015 at 16:14
  • 1
    \$\begingroup\$ It's interesting to see two very similar questions posted at the same time, with one OP helping the other... think you should deserve a special brownie for that. :) \$\endgroup\$
    – h.j.k.
    Feb 11, 2015 at 16:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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