patternjavaMinor
TicTacToe game in Java
Viewed 0 times
tictactoegamejava
Problem
I have made a simple Tic Tac Toe game but I think I have not followed a few conventions. I hope if you could help me improve it by suggesting some fixes.
```
package practice;
import javax.swing.*;
import java.awt.*;
import java.awt.event.*;
public class TicTacToe extends JFrame implements ActionListener{
/**
*
*/
private static final long serialVersionUID = 1L;
private JPanel panel;
private JButton button1,button2,button3,button4,button5,button6,button7,button8,button9;
private String letter = "";
private boolean win = false;
private int count = 0;
public static void main(String[] args){
new TicTacToe();
}
public TicTacToe(){
this.setSize(500,500);
this.setTitle("Tic Tac Toe");
this.setDefaultCloseOperation(EXIT_ON_CLOSE);
this.setLocationRelativeTo(null);
this.setResizable(false);
button1 = new JButton("");
button2 = new JButton("");
button3 = new JButton("");
button4 = new JButton("");
button5 = new JButton("");
button6 = new JButton("");
button7 = new JButton("");
button8 = new JButton("");
button9 = new JButton("");
panel = new JPanel();
panel.setLayout(new GridLayout(3,3));
panel.add(button1);
panel.add(button2);
panel.add(button3);
panel.add(button4);
panel.add(button5);
panel.add(button6);
panel.add(button7);
panel.add(button8);
panel.add(button9);
button1.addActionListener(this);
button2.addActionListener(this);
button3.addActionListener(this);
button4.addActionListener(this);
button5.addActionListener(this);
button6.addActionListener(this);
button7.addActionListener(this);
button8.addActionListener(this);
button9.addActionListener(this);
this.add(panel);
this.setVisible(true);
}
public void action
```
package practice;
import javax.swing.*;
import java.awt.*;
import java.awt.event.*;
public class TicTacToe extends JFrame implements ActionListener{
/**
*
*/
private static final long serialVersionUID = 1L;
private JPanel panel;
private JButton button1,button2,button3,button4,button5,button6,button7,button8,button9;
private String letter = "";
private boolean win = false;
private int count = 0;
public static void main(String[] args){
new TicTacToe();
}
public TicTacToe(){
this.setSize(500,500);
this.setTitle("Tic Tac Toe");
this.setDefaultCloseOperation(EXIT_ON_CLOSE);
this.setLocationRelativeTo(null);
this.setResizable(false);
button1 = new JButton("");
button2 = new JButton("");
button3 = new JButton("");
button4 = new JButton("");
button5 = new JButton("");
button6 = new JButton("");
button7 = new JButton("");
button8 = new JButton("");
button9 = new JButton("");
panel = new JPanel();
panel.setLayout(new GridLayout(3,3));
panel.add(button1);
panel.add(button2);
panel.add(button3);
panel.add(button4);
panel.add(button5);
panel.add(button6);
panel.add(button7);
panel.add(button8);
panel.add(button9);
button1.addActionListener(this);
button2.addActionListener(this);
button3.addActionListener(this);
button4.addActionListener(this);
button5.addActionListener(this);
button6.addActionListener(this);
button7.addActionListener(this);
button8.addActionListener(this);
button9.addActionListener(this);
this.add(panel);
this.setVisible(true);
}
public void action
Solution
Use arrays
The first thing that struck me is much of the code could be simplified through the use of arrays and complementary loops.
For example,
these lines:
Could be replaced with:
You can also do your tests for vertical, horizontal in a loop although you'd refer to the index rather than button1 or button2 if you use arrays, so remember that indexing starts at 0.
Extract more logic into methods
You want your tasks to be as modular as possible.
Your listener is responsible for:
This is a heavy workload and if a single thing breaks it may seemingly affect everything else, leading you to try and fix things that don't need fixing, and the compiler to even point at incorrect error locations.
Anytime you see a method or class instantiation and can't get a succinct answer for what it does, it probably does too much. You may not see this now, but if someone else looked at the code or if you came back to it in six months it would be more apparent. Read more about the Single Responsibility Principle.
The first thing that struck me is much of the code could be simplified through the use of arrays and complementary loops.
For example,
these lines:
button1 = new JButton("");
button2 = new JButton("");
button3 = new JButton("");
button4 = new JButton("");
button5 = new JButton("");
button6 = new JButton("");
button7 = new JButton("");
button8 = new JButton("");
button9 = new JButton("");
panel.add(button1);
panel.add(button2);
panel.add(button3);
panel.add(button4);
panel.add(button5);
panel.add(button6);
panel.add(button7);
panel.add(button8);
panel.add(button9);
button1.addActionListener(this);
button2.addActionListener(this);
button3.addActionListener(this);
button4.addActionListener(this);
button5.addActionListener(this);
button6.addActionListener(this);
button7.addActionListener(this);
button8.addActionListener(this);
button9.addActionListener(this);Could be replaced with:
JButton[] buttons = new JButton[9];
for (JButton button : buttons) {
button = new JButton("");
panel.add(button);
button.addActionListener(this);
}You can also do your tests for vertical, horizontal in a loop although you'd refer to the index rather than button1 or button2 if you use arrays, so remember that indexing starts at 0.
Extract more logic into methods
You want your tasks to be as modular as possible.
Your listener is responsible for:
- Processing event triggers.
- Altering game state.
- Evaluating win conditions.
- Displaying game end messages.
This is a heavy workload and if a single thing breaks it may seemingly affect everything else, leading you to try and fix things that don't need fixing, and the compiler to even point at incorrect error locations.
Anytime you see a method or class instantiation and can't get a succinct answer for what it does, it probably does too much. You may not see this now, but if someone else looked at the code or if you came back to it in six months it would be more apparent. Read more about the Single Responsibility Principle.
Code Snippets
button1 = new JButton("");
button2 = new JButton("");
button3 = new JButton("");
button4 = new JButton("");
button5 = new JButton("");
button6 = new JButton("");
button7 = new JButton("");
button8 = new JButton("");
button9 = new JButton("");
panel.add(button1);
panel.add(button2);
panel.add(button3);
panel.add(button4);
panel.add(button5);
panel.add(button6);
panel.add(button7);
panel.add(button8);
panel.add(button9);
button1.addActionListener(this);
button2.addActionListener(this);
button3.addActionListener(this);
button4.addActionListener(this);
button5.addActionListener(this);
button6.addActionListener(this);
button7.addActionListener(this);
button8.addActionListener(this);
button9.addActionListener(this);JButton[] buttons = new JButton[9];
for (JButton button : buttons) {
button = new JButton("");
panel.add(button);
button.addActionListener(this);
}Context
StackExchange Code Review Q#111334, answer score: 4
Revisions (0)
No revisions yet.