3
\$\begingroup\$

I am a self - taught coder, and have been learning Java for the last 2 years.

I have recently created a tic tac toe game in Java. I have rearranged the program into separate classes & methods. I am skeptical of how object-oriented my program is.

I don't really want help on how to make the game better in visuals or anything like that. I am asking those who are more advanced programmers to help me with implementing programming techniques, better organization, etc. Pretty much anything that will help me become a better programmer.

Also, any extra tips for my future projects would be much appreciated!

BoardButton.java

import javax.swing.JButton;

public class BoardButton extends JButton
{
    private String sign;
    private boolean pressed;
    private int xPos,yPos,value;

    public BoardButton(int xPos, int yPos) {
        this.xPos = xPos;
        this.yPos = yPos;
        value = 0;
    }
    public void setSign(String sign) {
        setText(sign);
    }
    public String getSign() {
       return getText();
    }
    public boolean getState() {
       return pressed;
    }
    public void setState(boolean pressed) {
        this.pressed = pressed;
    }
    public int getValue() {
        return value;
    }
    public void setValue(int value) {
        this.value = value;
    }        
}

GUI.java

import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JButton;
import javax.swing.JLabel;
import javax.swing.BoxLayout;
import javax.swing.JOptionPane;
import javax.swing.Box;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.GridLayout;
import java.awt.Font;
import java.awt.Color;
import java.awt.BorderLayout;
import java.awt.Component;

public class GUI extends JFrame {
    private JPanel mainPanel, scoreBoard, board, timerPanel;
    private JButton newGame, reset;
    private JLabel circle, cross, tie, scores, turn;
    private String winner;
    private BoardButton btn[][];
    private int clicks, circleCount, crossCount, tieCount;
    private Timer timer;

    public GUI() {
        board = new JPanel();
        mainPanel = new JPanel();
        scoreBoard = new JPanel();
        timerPanel = new JPanel();

        timer = new Timer();

        newGame = new JButton("New Game");
        reset = new JButton("Reset Scores");

        cross = new JLabel("Cross: 0 wins");
        circle = new JLabel("Circle: 0 wins");
        tie = new JLabel("Ties: 0");
        scores = new JLabel("Scores");
        turn = new JLabel("Circles turn");

        scores.setFont(new Font("Arial", Font.BOLD, 20));
        scores.setForeground(Color.darkGray);

        cross.setFont(new Font("Arial", Font.BOLD, 15));
        cross.setForeground(Color.lightGray);

        circle.setFont(new Font("Arial", Font.BOLD, 15));
        circle.setForeground(Color.lightGray);

        tie.setFont(new Font("Arial", Font.BOLD, 15));
        tie.setForeground(Color.lightGray);

        scoreBoard.setLayout(new BoxLayout(scoreBoard, BoxLayout.Y_AXIS));

        scoreBoard.add(Box.createVerticalStrut(10));
        scoreBoard.add(scores);
        scoreBoard.add(circle);
        scoreBoard.add(Box.createVerticalStrut(20));
        scoreBoard.add(cross);
        scoreBoard.add(Box.createVerticalStrut(20));

        scoreBoard.add(tie);
        scoreBoard.add(reset);
        scoreBoard.add(turn);

        timerPanel.add(timer);
        timerPanel.add(newGame);

        mainPanel.setLayout(new BorderLayout());
        mainPanel.add(board, BorderLayout.CENTER);
        mainPanel.add(scoreBoard, BorderLayout.EAST);
        mainPanel.add(timerPanel, BorderLayout.SOUTH);

        add(mainPanel);

        newGame.addActionListener(new BtnListener());
        reset.addActionListener(new BtnListener());

        btn = new BoardButton[3][3]; 
        board.setLayout(new GridLayout(3,3));
        timer.setRunning(true);

        for(int i=0; i<3; i++){
            for(int j=0; j<3; j++) {
               btn[i][j]=new BoardButton(j,i);
               btn[i][j].setFont(new Font("Arial", Font.BOLD, 70));
               btn[i][j].setForeground(Color.blue);
               btn[i][j].addActionListener(new BoardListener());
               board.add(btn[i][j]);
            }
        }         
    } 

    //Checks for winner
    public void checkWin() {
        int diagSum1 = 0;
        int diagSum2 = 0;
        int colSum = 0;
        int rowSum = 0;
        String winner = "";

        diagSum1 = btn[0][2].getValue() + btn[1][1].getValue() + btn[2][0].getValue();
        diagSum2 = btn[0][0].getValue() + btn[1][1].getValue() + btn[2][2].getValue();

        if(diagSum1 == 3 || diagSum2 == 3) {
            winner = "Cross";
            crossCount++;
        }
        else if(diagSum1 == -3 || diagSum2 == -3) {
            winner = "Circle";
            circleCount++;
        }

        for(int i = 0; i<3; i++) {
            for(int j = 0; j<3; j++) {
                rowSum += btn[i][j].getValue(); 
                colSum += btn[j][i].getValue();
            }
            if(rowSum == 3 || colSum == 3 && winner.equals("")) {                 
                winner = "Cross";                                     
                crossCount++;
            }
            else if(rowSum == -3 || colSum == -3 && winner.equals("")) {
                winner = "Circle";
                circleCount++;
            }
            rowSum = 0;
            colSum = 0;
        }
        if(clicks == 9 && winner.equals("")) {
            winner = "No one";
            tieCount++;
        }
        if(!winner.equals("")) {
            setPanelEnabled(board, false);
            timer.setRunning(false);  
            turn.setText("");
            JOptionPane.showMessageDialog(null, winner + " is the winner!","Results",-1); 
        }
        updateScores();
    }

    public void reset() {
        for(int i=0; i<3; i++){
            for(int j=0; j<3; j++) {
               btn[i][j].setSign("");
               btn[i][j].setState(false);
               btn[i][j].setValue(0);
            }
        }          
    }

    public void setPanelEnabled(JPanel panel, Boolean isEnabled) {
        panel.setEnabled(isEnabled);
        Component[] components = panel.getComponents();

        for(int i = 0; i < components.length; i++) {
            if(components[i].getClass().getName() == "javax.swing.JPanel") {
                setPanelEnabled((JPanel) components[i], isEnabled);
            }

            components[i].setEnabled(isEnabled);
        }
    }   

    public void updateScores() {
        circle.setText("Circle: " + Integer.toString(circleCount) + " wins");
        cross.setText("Cross: " + Integer.toString(crossCount) + " wins");
        tie.setText("Ties: " + Integer.toString(tieCount));
    }

    class BoardListener implements ActionListener {
        public void actionPerformed (ActionEvent e) {
            BoardButton buttonClicked = (BoardButton)e.getSource();
            if(buttonClicked.getState()==false) {
                clicks++;
                if(clicks%2==0) {
                    buttonClicked.setText("X");
                    buttonClicked.setForeground(Color.blue);
                    buttonClicked.setValue(1);                    
                    turn.setText("Circles turn");
                    checkWin();
                }
                else {
                    buttonClicked.setText("O");
                    buttonClicked.setValue(-1);
                    buttonClicked.setForeground(Color.red);
                    turn.setText("Cross turn");
                    checkWin();
                }
            }  
            buttonClicked.setState(true);
        }            
    }  

    class BtnListener implements ActionListener {
        public void actionPerformed (ActionEvent e) {
            if(e.getSource() == newGame) {
                reset();
                setPanelEnabled(board, true); 
                timer.setRunning(true);
                timer.getClock().resetTime();
                    timer.getDisplay().setText(timer.getClock().getCurrentDisplayTime()); 
                turn.setText("Circles turn");
                clicks = 0;
            } 
            else if(e.getSource() == reset) {
                circleCount = 0;
                crossCount = 0;
                tieCount = 0;
                updateScores();
            }
        }
    }
}

TicTacToeMain.java

public class TicTacToeMain
{
    public static void main(String[] args) {
        GUI g = new GUI();
        g.setTitle("TicTacToe");
        g.setSize(500,475);
        g.setVisible(true);
        g.setLocationRelativeTo(null);
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ I've rolled back your edit because it essentially invalidates the existing answer. For an iterative review, you should post a new question, possibly linking back to this one for context. \$\endgroup\$ – forsvarir Dec 15 '16 at 16:41
2
\$\begingroup\$

Naming

Good naming is hard to get right, however it's worth taking the extra time because it makes your code a lot easier to follow. This applies to all elements of your program, albeit classes, methods, variables etc. GUI is a very generic title for a class. What does it really represent, TicTacToeFrame? You have a similar issue with some of your method names. Looking at your BoardButton class, it's got three properties 'Sign', 'State' and 'Value'. On their own, they mean nothing to me. It's not really obvious which property might represent the 'X' or 'O'. It could be Value, it could be Sign. The process of coming up with a good name that encapsulates the responsibility can also help to flag up when an element is responsible for too much.

main

In your main, you're doing things like setting the title and size on your GUI. This seems like it should really have been done in the GUI constructor. The GUI knows it's a TicTacToe game and is pretty tied to it, why does main have to tell it?

Gui or Game

You've implemented your entire game within your GUI class. This means that if you were to decide that you wanted to implement a console version / a web version, you'd be starting again. It's generally a good idea to try and break your application into different layers. A good starting split would be to separate the display logic (show the board, let users pick where they want to play, provide a means to feedback the winner), and the game logic (maintain internal board structure, expose methods to make a move, the actual logic that checks for a win).

You could then break these layers down further is required (it could make sense for example to have a scoreboard class to keep track of the running scores).

Having this separation has several benefits. It means that if you decide to plug in a different front end, you just have to worry about the front end. The mechanics of the game could be reused. It also means that if you want to, you can write untit tests for the logic to validate different scenarios.

Winner validation

Your winner validation is straight forward and can be called at any time. This isn't bad, however you're probably making more checks than you need to. When you place an individual piece, you only actually need to check rows/columns/diagonals that the piece is on. If you place it in column 1, then it won't have impacted column 3.

\$\endgroup\$
  • \$\begingroup\$ I have tried to implement what you said however, i have a problem. I tried to create a Game class that incorporates all game-logic but some of the variables i am using. Like the int that is counting whos turn it is, it needs to be accessed from like every class. Should i make those variables static? \$\endgroup\$ – samuel tober Dec 14 '16 at 21:01
  • \$\begingroup\$ @samueltober making the variables static isn't ideal, statics can get messy. However, if you are able to get the code working by using them, then it may be worth you posting a follow up question with the new code. You may then get advice on how you could evolve your new structure and class interactions. There are also other examples on the site, so you might want to look at how they solve the same game. For example codereview.stackexchange.com/questions/71756/tic-tac-toe-in-mvc \$\endgroup\$ – forsvarir Dec 14 '16 at 21:32

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.