Object Oriented TicTacToe

Question

I have been trying to learn to program in a more object oriented manner and I am at the moment working on a Tic Tac Toe game. I have tried to split my classes so that it would make sense, OOP wise. I am just concerned whether I have split it up correctly and if I am really following the standards of OOP programming. I am especially concerned about the fact that I am creating a Scoreboard object inside GameBoard in order to access the methods of scoreboard. I'm not sure if there is a more neat way to do this.

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;
    }        
}

ScoreBoard.java

import javax.swing.JPanel;
import javax.swing.JButton;
import javax.swing.JLabel;
import javax.swing.Box;
import java.awt.Font;
import java.awt.Color;
import javax.swing.BoxLayout;
import java.awt.event.ActionListener;
import java.awt.event.ActionEvent;

public class Scoreboard extends JPanel
{
    private JButton reset, newGame;
    private JLabel circle, cross, tie, scores;

    public Scoreboard() {
        newGame = new JButton("New Game");
        reset = new JButton("Reset Scores");
        reset.addActionListener(new BtnListener());
        newGame.addActionListener(new BtnListener());

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

        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);

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

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

        add(tie);
        add(reset);
        add(Box.createVerticalStrut(100));
    }  
    public void updateScore(int circleScore, int crossScore, int tieScore) {
        circle.setText("Cirlcle: " + Integer.toString(circleScore) + " wins");
        cross.setText("Cross: " + Integer.toString(crossScore) + " wins");
        tie.setText("Ties: " + Integer.toString(tieScore));   
    }
    class BtnListener implements ActionListener {
        public void actionPerformed (ActionEvent e) {
            if(e.getSource() == reset) {
                circle.setText("Circle: 0 wins");
                cross.setText("Cross: 0 wins");
                tie.setText("Ties: 0");
            }
        }
    }    
}

GameBoard.java

import javax.swing.JPanel;
import java.awt.Font;
import java.awt.Color;
import java.awt.event.ActionListener;
import java.awt.event.ActionEvent;
import javax.swing.JOptionPane;
import java.awt.Component;
import java.awt.GridLayout;

public class GameBoard extends JPanel
{
    private BoardButton[][] btn;
    private int crossCount, circleCount, tieCount, clicks;
    private boolean win;
    private Timer timer;
    private Scoreboard scoreboard;

    public GameBoard(Scoreboard scoreboard, Timer timer) {
        win = false;
        this.scoreboard = scoreboard;
        this.timer = timer;
        btn = new BoardButton[3][3];

        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());
               add(btn[i][j]);
            }
        }  
        setLayout(new GridLayout(3,3));
    }

    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++;
            win = true;
        }
        else if(diagSum1 == -3 || diagSum2 == -3) {
            winner = "Circle";
            circleCount++;
            win = true;
        }

        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++;
                win = true;
            }
            else if(rowSum == -3 || colSum == -3 && winner.equals("")) {
                winner = "Circle";
                circleCount++;
                win = true;
            }
            rowSum = 0;
            colSum = 0;
        }
        if(clicks == 9 && winner.equals("")) {
            winner = "No one";
            win = true;
            tieCount++;
        }
        if(win) {
            setPanelEnabled(this, false);
            timer.setRunning(false);
            JOptionPane.showMessageDialog(null, winner + " is the winner!","Results",-1); 
        }
        scoreboard.updateScore(circleCount, crossCount, tieCount);
    }

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

    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);
        }
    }

    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);                    
                    checkWin();
                }
                else {
                    buttonClicked.setText("O");
                    buttonClicked.setValue(-1);
                    buttonClicked.setForeground(Color.red);
                    checkWin();
                }
            }  
            buttonClicked.setState(true);
        }            
    }    
}

TicTacToeGUI.java

import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JButton;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.BorderLayout;
import javax.swing.JLabel;

public class TicTacToeGUI extends JFrame {
    private JPanel mainPanel;
    private Timer timer;
    private Scoreboard scoreboard;
    private GameBoard game;
    private JButton newGame;

    public TicTacToeGUI() {
        mainPanel = new JPanel();
        timer = new Timer();
        scoreboard = new Scoreboard();
        game = new GameBoard(scoreboard, timer);

        newGame = new JButton("New Game");
        newGame.addActionListener(new BtnListener());
        scoreboard.add(newGame);

        mainPanel.setLayout(new BorderLayout());
        mainPanel.add(game, BorderLayout.CENTER);
        mainPanel.add(scoreboard, BorderLayout.EAST);
        mainPanel.add(timer, BorderLayout.SOUTH);

        add(mainPanel);
        timer.setRunning(true);  

        setTitle("TicTacToe");
        setSize(500,475);
        setLocationRelativeTo(null);  
        setVisible(true);    
    }

    class BtnListener implements ActionListener {
        public void actionPerformed (ActionEvent e) {
            if(e.getSource() == newGame) {
                game.reset();
                game.setPanelEnabled(game, true);
                timer.setRunning(true);
            }
        }
    }      
}

TicTacToeMain.java

public class TicTacToeMain
{
    public static void main(String[] args) {
        TicTacToeGUI g = new TicTacToeGUI();
    }
}

the problem with this is that OOP offers no benefit to this program. All game data and logic can be handled with a couple arrays and functions. Its impossible to KISS with OOP, because a functional / procrdural version of this code would be three times shorter, and thread safe. — Fluidity
– Fluidity, Commented Dec 19, 2016 at 13:50
getter and setters that do nothing except get and set seem particularly silly to me. why don't you just modify pr access the property :)? there is literally no difference, except you make more busywork for yourself with useless functions especially in BoardButton — Fluidity
– Fluidity, Commented Dec 19, 2016 at 13:53
but the issue is with the paradigm, not your code in particular, which seems fine... its just unnecessarily long and complex due to OOP. Gui should be your class, and game data as a struct inside. 1 Buttons modify your struct. 2 Check at end to see who won 3 update Gui .. repeat xD 3 step procedural TicTacToe, one class, third of the code, and one file not four :) which is inherently easier to maintain and debug — Fluidity
– Fluidity, Commented Dec 19, 2016 at 14:06

Timothy Truckle · Accepted Answer · 2016-12-18 10:29:47Z

I have tried to split my classes so that it would make sense, OOP wise

OOP doesn't mean to "split up" code into random classes.

Doing OOP means that you follow certain principles which are (amongst others):

information hiding / encapsulation
single responsibility
separation of concerns
KISS (Keep it simple (and) stupid.)
DRY (Don't repeat yourself.)
"Tell! Don't ask."
Law of demeter ("Don't talk to strangers!")

In Java the classes concept supports this principles but OOP is not bound to it.

I think your code does not reflect this principles. – Timothy Truckle 14 hours ago

Why not? What can i change? – samuel tober

Your major concern should be the business code.

So I'd start by introducing a CellPosition class holding x and y coordinates and a Direction class that can create a new CellPosition from a given one as needed.
Then I'd create lines by associating a CellPosition (start point) with a Direction in a Map.
Finally I'd create a method than takes a CellPosition and a Direction and checks if all cells in than direction belong to the same player..

The relevant parts would look like this:

class  CellPosition {
  private final int x, y; 
  CellPosition(int x, int y){
    this.x  = x;
    this.y  = y;
  }
  public int getX(){return x;}
  public int getY(){return y;}
}

interface Direction {
  CellPosition next(CellPosition current);
}

enum Player{NONE,P1,P2};

class GameEndChecker{
  static class PlayerWon entends Exception {
  // IMHO winning the game is an Exception with respect to the normal game flow
  // since we have two "game end" conditions the calling code would become
  // more complex when using a more complex return state to avoid the exception.
  // this does not mean that an Exception is always appropriate to signal a winner...
      private final Player winner;
      PlayerWon(Player winner){ this.winner = winner;
      @Override
      public String getMessage(){
         return "winner is: Player "+winner;
      }
  }
  private final Player[][] board;
  private final Map<CellPosition,Direction> lines;
  GameEndChecker(Player[][] board){
     this.board  =  board;
      addHorizontalLines(lines);
      addDiagonalLines(lines);
      addVerticalLines(lines);
  }

  private addHorizontalLines(Map<CellPosition,Direction> lines){
     for(int i =0, i< board.length, i++){
         lines.put(new CellPosition(i,0), cellPosition-> new CellPosition(0,cellPosition.getY()+1));
     }
  }

  private addDiagonalLines(Map<CellPosition,Direction> lines){
     lines.put(new CellPosition(0,0), cellPosition-> new CellPosition(cellPosition.getX()+1,cellPosition.getY()+1));
     lines.put(new CellPosition(2,0), cellPosition-> new CellPosition(cellPosition.getX()-1,cellPosition.getY()+1));
  }

  private addVerticalLines(Map<CellPosition,Direction> lines){
     for(int i =0, i< board.length, i++){
         lines.put(new CellPosition(0,i), cellPosition-> new CellPosition(cellPosition.getX()+1,0));
     }
  }

  public boolean isGameEnd() throws PlayerWon{
    boolean hasFreeFields=false;
    for(Map.Entry<CellPosition,Direction>line : lines){
       CellPosition currentCell = line.getKey();
       // Sets hold unique values, doubles get filtered out.
       Set<Player> lineOwners = new HashSet();
       for(int i =0, i< board.length, i++){
          Player fieldOwner= board[currentCell.getX()][currentCell.getY()]
          currentCell =  line.value().next(currentCell);
          lineOwners.add(fieldOwner);
          if(Player.NONE.equals(fieldOwner))
             hasFreeFields=true;
       }
       if(1==lineOwners.size()&& !lineOwners.contains(Player.NONE))
         // one player owns a complete line.
         throw new PlayerWon(lineOwners.iterator().next());
    }
    return !hasFreeFields;
  }
}

Is it okay to instantiate an object inside a class [] – samuel tober

In java, there is no code outside a class. Therefore the question should be Is it okay to instantiate an object inside an object of a different class?

in order to access that object's methods? Say for example that I create a Scoreboard inside the GameBoard class only to be able to access Scoreboard's methods, would that be considered "good practice"? – samuel tober

You will find many programmers saying "Yes, of cause."

But my understanding is different.

Classes in general fall into 2 logical types: Data Transfer objects (DTOs) and others.

DTOs are special classes that have no (business) logic of their own, just getters. Usually they also have setters, but IMHO they soudn't unless required by a framework...

Classes of the other type provide business logic that works on data passed in via method Parameters (most likely in form of DTOs). Objects of such classes sometimes need to collaborate with other objects, so called dependencies. IMHO the instantiation of dependencies is NOT a responsibility of a class (Separation of concerns). Therefore any object should not instantiate the dependencies it works with itself.
Nevertheless it may instanciate dependensies for the objects it colaborates, but the better way is to use a dependency injection framework.

Your major concern should be the business code. So I'd start by introducing a 'CellPosition' class holding x and y coordinates and a Direction class that can create a new 'CellPosition' from a given one as needed. Then I'd associate start with CellPositions with Direction in a Map<CellPosition,Direction>. finaly I'd create a method than takes a CellPosition and a Direction that checks if all cells in than direction belong to the same player. Of cause there are other valid OOP approaches to that problem... — Timothy Truckle
– Timothy Truckle, Commented Dec 16, 2016 at 10:44
As it stands, this answer is more of an elaborate comment. Please edit to review OP's code and relate your statements to the OP. Cheers! — Mathieu Guindon
– Mathieu Guindon, Commented Dec 16, 2016 at 21:16
I think people generally try to avoid using exceptions for flow control, which is what you're doing. However, using an exception to indicate that a player has won the game is an interesting approach. — forsvarir
– forsvarir, Commented Dec 17, 2016 at 18:33
Thanks!!! However, I still have a question. Is it okay to instantiate an object inside a class in order to access that object's methods? Say for example that I create a Scoreboard inside the GameBoard class only to be able to access Scoreboard's methods, would that be considered "good practice"? — Unknown
– Unknown, Commented Dec 17, 2016 at 20:46

Stack Exchange Network

Object Oriented TicTacToe

1 Answer 1

Your Answer

Hot Network Questions

Object Oriented TicTacToe

1 Answer 1

Your Answer

Sign up or log in

Post as a guest

Related

Hot Network Questions