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 '15 at 10:57
  • \$\begingroup\$ What version of Java are you using? \$\endgroup\$ – barq Feb 12 '15 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\$ – TheQuantumBros Feb 13 '15 at 8:53
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\$ – Daryl Beaumont Feb 11 '15 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 '15 at 16:44

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.