HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

2 Player TicTacToe Game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
playergametictactoe

Problem

I am a beginner Java programmer and I have just finished creating a simple 2 player TicTacToe game and would appreciate some feedback/advice on how to make it better.

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

public class TicTacToe implements ActionListener {

public static int turnNum;
public static JLabel title;
public static JButton[][] buttons = new JButton[3][3];
public static JButton reset;
public static boolean xTurn = true;
public static boolean won = false;
public static int[][] grid = new int[3][3];

public static void main(String[] args) {
// Frame
JFrame frame = new JFrame("TicTacToe");
frame.setSize(255, 234);
frame.setLocationRelativeTo(null);
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
frame.setResizable(false);

// Container
JPanel container = new JPanel();
container.setLayout(null);
frame.setContentPane(container);

// Contents
// Title
title = new JLabel("TicTacToe");
title.setBounds(55, 10, 70, 15);

// Buttons
// Loop to create add all buttons to array and to add action listeners to all
for (int a = 0; a < buttons.length; a++) {
for (int b = 0; b < buttons.length; b++) {
buttons[a][b] = new JButton("_");
buttons[a][b].addActionListener(new TicTacToe());
}
}
// Row 1
buttons[0][0].setBounds(5, 40, 50, 50);
buttons[0][1].setBounds(60, 40, 50, 50);
buttons[0][2].setBounds(115, 40, 50, 50);
// Row 2
buttons[1][0].setBounds(5, 95, 50, 50);
buttons[1][1].setBounds(60, 95, 50, 50);
buttons[1][2].setBounds(115, 95, 50, 50);
// Row 3
buttons[2][0].setBounds(5, 150, 50, 50);
buttons[2][1].setBounds(60, 150, 50, 50);
buttons[2][2].setBounds(115, 150, 50, 50);

reset = new JButton("Reset");
reset.setBounds(170, 55, 75, 20);
reset.addActionListener(new TicTacToe());

// Adding contents
container.add(title);

Solution

OOP

Use different (or the same) classes. Put everything in a class and replace your new TicTacToe(); with this. Java is not for functional programming

Buttons

Your buttons[x][y].setBound(...) can be integrated in the loop, as well as container.add(buttons[a][b]);

container.add(title);
for (int a = 0; a < buttons.length; a++) {
  for (int b = 0; b < buttons.length; b++) { 
    buttons[a][b] = new JButton("_"); 
    buttons[a][b].addActionListener(this);
    buttons[a][b].setBounds(5+(a*55),40+(55*b), 50, 50);
    container.add(buttons[a][b]);
  } 
}


actionPerformed

public void actionPerformed(ActionEvent e){
  if(e.getSource==reset){
    //do reset stuff
    return;
  }
  if(xTurn){
    e.getSource().setTitle("X");
    e.getSource().setEnabled(false);
    xTurn=false;
  }else{
    //same with "O"
  }
  turnNum++;
}


I replaces your grid with the buttons. If you want to look up who owns which fields use buttons[a][b].getTitle().equals("X"/"O").

hasWon

If you use the use the buttons to check who owns which fields I recommend do an extra method to check per letter if this letter hasWon()

public boolean hasWon(String letter){
  for(int i=0;i<buttons.length;i++){
    if(buttons[i][0].getTitle().equals(letter)&&buttons[i][1].getTitle().equals(letter)&&buttons[i][2].getTitle().equals(letter))return true;
    if(buttons[0][i].getTitle().equals(letter)&&buttons[1][i].getTitle().equals(letter)&&buttons[2][i].getTitle().equals(letter))return true;
  }
  if(buttons[0][0].getTitle().equals(letter)&&buttons[1][1].getTitle().equals(letter)&&buttons[2][2].getTitle().equals(letter))return true;
  if(buttons[0][2].getTitle().equals(letter)&&buttons[1][1].getTitle().equals(letter)&&buttons[2][0].getTitle().equals(letter))return true;
  return false;
}
public boolean hasWon(){
  if(hasWon("X")){
    //X has Won
  }else if(hasWon("O")){
    //O has Won
  }else{
    return false;
  }
  return true;
}


Still TicTacToe#hasWon(String) is massive, but I don't know how to do this smarter. :(

Code Snippets

container.add(title);
for (int a = 0; a < buttons.length; a++) {
  for (int b = 0; b < buttons.length; b++) { 
    buttons[a][b] = new JButton("_"); 
    buttons[a][b].addActionListener(this);
    buttons[a][b].setBounds(5+(a*55),40+(55*b), 50, 50);
    container.add(buttons[a][b]);
  } 
}
public void actionPerformed(ActionEvent e){
  if(e.getSource==reset){
    //do reset stuff
    return;
  }
  if(xTurn){
    e.getSource().setTitle("X");
    e.getSource().setEnabled(false);
    xTurn=false;
  }else{
    //same with "O"
  }
  turnNum++;
}
public boolean hasWon(String letter){
  for(int i=0;i<buttons.length;i++){
    if(buttons[i][0].getTitle().equals(letter)&&buttons[i][1].getTitle().equals(letter)&&buttons[i][2].getTitle().equals(letter))return true;
    if(buttons[0][i].getTitle().equals(letter)&&buttons[1][i].getTitle().equals(letter)&&buttons[2][i].getTitle().equals(letter))return true;
  }
  if(buttons[0][0].getTitle().equals(letter)&&buttons[1][1].getTitle().equals(letter)&&buttons[2][2].getTitle().equals(letter))return true;
  if(buttons[0][2].getTitle().equals(letter)&&buttons[1][1].getTitle().equals(letter)&&buttons[2][0].getTitle().equals(letter))return true;
  return false;
}
public boolean hasWon(){
  if(hasWon("X")){
    //X has Won
  }else if(hasWon("O")){
    //O has Won
  }else{
    return false;
  }
  return true;
}

Context

StackExchange Code Review Q#145054, answer score: 3

Revisions (0)

No revisions yet.