Conways Game of Life in Swing, using OOP

Question

I'm relatively new to coding and was wondering if anyone could review my code? I'm trying to get my head around the concepts of Object-oriented programming in particular. This is an implementation of Conway's Game of Life I was assigned and any constructive feedback would be great. It works fine but I'd like to try and adhere to best practises as much as possible so I know for the future.

Main:

public class Main 
{
    public static void main(String[] args) 
    {
        GUIFrame golgui = new GUIFrame(); 
        javax.swing.SwingUtilities.invokeLater(golgui);
    }
}

GUIFrame - basically just sets up the JFrame:

import javax.swing.JFrame;

public class GUIFrame implements Runnable
{

    public void run() 
    {
         //Create and set up the Window
        JFrame frame = new JFrame("Game of Life");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        //Create and set up the content Pane based on our MainPanel class
        MainPanel newContentPane = new MainPanel();
        //All content Panes must be opaque apparently...
        newContentPane.setOpaque(true);
        //Put the Pane in the Window!
        frame.setContentPane(newContentPane);
        //Move to point 250,250 (offset from top left) and size to 400,400 pixels
        frame.setBounds(10,10,775,790);
        frame.setResizable(false);
        frame.pack();
        //Display the frame.
        frame.setVisible(true);
    }
}

MainPanel - to add other JPanel classes to

import javax.swing.JPanel;
import java.awt.BorderLayout;

public class MainPanel extends JPanel 
{

    private static final long serialVersionUID = 1862962349L;

    private static Board board;
    private static Nav nav;
    private static Counter counter;

    //Constructor for our form
    public MainPanel() 
    {
        //Main panel, set to border layout
        setLayout(new BorderLayout());
        //Add Nav sub-panel to main panel
        nav = new Nav();
        board = new Board();
        counter = new Counter();

        //Add all the panels with their respective content to the different areas of the main JPanel
        add(nav,BorderLayout.NORTH);
        add(board,BorderLayout.CENTER);
        add(counter,BorderLayout.SOUTH);
    }

    public static Board getBoard() {
        return board;
    }

    public static Counter getCounter(){
        return counter;
    }
}

Nav - a JPanel that takes care of the controls

import java.awt.Color;
import java.awt.GridLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.JButton;
import javax.swing.JPanel;

public class Nav extends JPanel implements ActionListener{

    private static final long serialVersionUID = 573248957204985L;
    private JButton run = new JButton("Start");
    private JButton pause = new JButton("Pause");
    private JButton reset = new JButton("Reset");

    Nav(){

        setLayout(new GridLayout(1,3,0,0));

        run.setBackground(Color.GREEN); //Set button colour to green
        run.setFont(run.getFont().deriveFont(20.0f)); //Set font size to 20
        run.setActionCommand("Start"); //Set ActionCommand and listener
        run.addActionListener(this);

        pause.setBackground(Color.ORANGE); //Set colour to orange
        pause.setFont(pause.getFont().deriveFont(20.0f)); //Set font size to 20
        pause.setEnabled(false); //Don't allow it to be click-able initially
        pause.setActionCommand("Pause");
        pause.addActionListener(this);

        reset.setBackground(Color.RED); //Set colour to red
        reset.setFont(reset.getFont().deriveFont(20.0f)); //Set font size to 20
        reset.setEnabled(false); //Don't allow it to be click-able initially
        reset.setActionCommand("Reset");
        reset.addActionListener(this);

        add(run);
        add(pause);
        add(reset);

    }

    @Override
    public void actionPerformed(ActionEvent e) {

        Board board = MainPanel.getBoard();
        Counter counter = MainPanel.getCounter();

        if(e.getActionCommand()=="Start"){
            //Once started disable start button and enable others.
            run.setEnabled(false); 
            pause.setEnabled(true); 
            reset.setEnabled(true); 
            //Start timer in Board class to start triggering events
            board.getTimer().start();
        }
        if(e.getActionCommand()=="Pause"){
            pause.setEnabled(false); //Make pause button no longer selectable
            run.setEnabled(true); //Enable run button to start program again
            board.getTimer().stop(); //Stop timer in Board class from triggering events
        }
        if(e.getActionCommand()=="Reset"){
            //Reset buttons to original states
            reset.setEnabled(false); 
            pause.setEnabled(false);
            run.setEnabled(true);
            board.reset();
            counter.resetCount();
        }
    }
}

Board - handles the initial creation of the board and 'Clicked' and 'Timer' events

import java.awt.Color;
import java.awt.Dimension;
import java.awt.GridLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.Random;

import javax.swing.JButton;
import javax.swing.JPanel;
import javax.swing.Timer;

public class Board extends JPanel implements ActionListener{

    private static final long serialVersionUID = 2324324543246L;
    //Size of board
    private int column=25,row=25;
    //2D JButton Array make grid of cells
    private  JButton[][] cells = new JButton[row][column]; 
    private Timer timer;

    Board(){    
        setLayout(new GridLayout(column,row,0,0));
        //Populate cells with default settings
        for(int i = 0; i<row;i++)
        {
            for(int j=0;j<column;j++)
            {
                cells[i][j] = new JButton(); 
                cells[i][j].setBackground(Color.WHITE); 
                cells[i][j].setPreferredSize(new Dimension(20,20));
                cells[i][j].setActionCommand("Clicked");
                cells[i][j].addActionListener(this);
                add(cells[i][j]); 
            }
        }
        //Create new timer to trigger events every second
        timer = new Timer(1000, this); 
        timer.setActionCommand("Timer");
        timer.setInitialDelay(0); 
    }
    public JButton[][] getCells() {
        return cells;
    }
    public void setCells(JButton[][] cells) {
        this.cells = cells;
    }

    public Timer getTimer() {
        return timer;
    }

    public int getColumn() {
        return column;
    }
    public int getRow() {
        return row;
    }

    @Override
    public void actionPerformed(ActionEvent e) {

        if (e.getActionCommand().equals("Clicked")) {

            //Find cell clicked and make changes outlined in selectedCells method
            for(int i = 0; i<row;i++){
                for(int j=0;j<column;j++){

                    if(e.getSource()==cells[i][j]){
                        selectedCells(cells[i][j]);
                    }
                }
            }
        }
        if(e.getActionCommand().equals("Timer")){

            //If starting grid is empty create random grid
            if(arrayCheck()==true){
                randomizeCells();
            }
            //Each timer event increments counter...
            MainPanel.getCounter().incrementCount();
            //...and changes cells states as per rules of Game of Life
            NewBoard newBoard = new NewBoard();
        }
    }

    public void reset(){
        //Resets all cells back to original state
        for(int i = 0; i<row;i++){
            for(int j=0;j<column;j++){
                cells[i][j].setSelected(false); 
                cells[i][j].setBackground(Color.WHITE);
            }
        }
        timer.stop();
    }

    private void selectedCells(JButton cell){
        //Changes cell to selected and blue when clicked
        if(cell.isSelected()==false){
            cell.setBackground(Color.BLUE);
            cell.setSelected(true);
        }
        //If cell is already selected change to white and selected false
        else{
            cell.setBackground(Color.WHITE);
            cell.setSelected(false);
        }
    }

    //Creates the random grid if starting grid is empty
    private void randomizeCells(){

        for(int i = 0; i<row;i++){
            for(int j=0;j<column;j++){

                Random cellRandom = new Random();
                if(cellRandom.nextBoolean()==false){
                    cells[i][j].setSelected(false);
                    cells[i][j].setBackground(Color.WHITE);
                }
                else{
                    cells[i][j].setSelected(true);
                    cells[i][j].setBackground(Color.BLUE);
                }
            }
        }
    }

    //Checks whether grid is empty or not. Returns false if not.
    private boolean arrayCheck(){

        for(int i=0;i<row;i++){
            for(int j=0;j<column;j++){
                if(cells[i][j].isSelected()==true){
                    return false;
                }
            }
        }
        return true;
    }
}

NewBoard - calculates what the new board should look like for next iteration and sets Board to it

import java.awt.Color;

import javax.swing.JButton;

public class NewBoard {

    private Board board = MainPanel.getBoard();
    private JButton[][] cells = board.getCells();
    private int row = board.getRow(), column = board.getColumn();
    private boolean newCells[][] = new boolean[row][column];

    NewBoard(){
        neighbours(); 
    }

    //Checks how many neighbours each cell has (wraps around)
    private void neighbours(){
        for(int i = 0; i<row;i++)
        {
            for(int j=0;j<column;j++)
            {
                int neighbours=0;
                for(int x = -1; x<2;x++){
                    for(int y=-1;y<2;y++){
                        if(x==0 && y==0);
                        else if(cells[(i+x+column)%column][(j+y+row)%row].isSelected()==true)
                        {
                            neighbours++;
                        }
                    }
                }
                /*
                 * Store new cell in newCells boolean array, setCellStatus checks rules for cell life/death
                 * We need a separate array as otherwise as program loops through, previous cells would have 
                 * changed before other cells have been checked against the rules. Need board to change as one.
                 */
                newCells[i][j] = setCellStatus(cells[i][j],newCells[i][j],neighbours);
            }
        }
        createNewBoard();
    }

    //Sets what the current cell should be for the new board depending on Game of Life rules
    private boolean setCellStatus(JButton cell, boolean newCell, int neighbours){

        if(cell.isSelected()==true && ((neighbours < 2) || (neighbours > 3))) newCell=false;

        else if(cell.isSelected()==false && (neighbours==3)) newCell=true;

        else if(cell.isSelected()==true) newCell=true;

        else{
            newCell=false; //Otherwise remain false
        }
        return newCell; //Return outcome to newCells index
    }

    //Creates and sets the new board based on boolean array newCells
    private void createNewBoard(){

        for(int i = 0; i<row;i++){
            for(int j = 0; j<column;j++){

                if(newCells[i][j]==true){
                    cells[i][j].setSelected(true);
                    cells[i][j].setBackground(Color.BLUE);
                }
                else{
                    cells[i][j].setSelected(false);
                    cells[i][j].setBackground(Color.WHITE);
                }
            }
        }
        board.setCells(cells);
    }
}

Counter - simply is a label at the bottom of the board that counts the number of iterations the game is on

import javax.swing.JLabel;
import javax.swing.border.EmptyBorder;

public class Counter extends JLabel{

    private static final long serialVersionUID = 3698564745876L;
     int count = 0;

    Counter(){

        //Creates the label's look and position
        setOpaque(true);
        setText("Iteration");
        setHorizontalAlignment(JLabel.CENTER);
        setFont(getFont().deriveFont(40.0f)); //Set counter size
        setBorder(new EmptyBorder(10,10,10,10));

    }

    public void incrementCount(){
        setText(String.valueOf(count++));
    }

    public void resetCount(){
        count = 0;
        setText("Iteration");
    }

}

Answer 1

Just some quick notes:

• Having a minimal Main class is a great starting point.

• You should use whitespace consistently, e.g. in for (int i = 0; i<length;i++). Just let your IDE format the source code automatically. The exact style rules don't matter, it's more important that the whole project follows the same style.

• Do not use static fields, e.g. in MainPanel. Your application currently only has one MainPanel, but it is not part of the concept of the MainPanel that all main panels in the world share the same nav and board. Yet this is what the static keyword is about.

• GUIFrame should be renamed to Application, since it is not a frame but a Runnable. You cannot run a frame, but you can surely run an application.

• Do not compare string literals with the == operator, e.g. in Nav.actionPerformed. In your case it works, but this is not guaranteed anymore when you translate your application into different languages.

• In Nav.actionPerformed, you can use a switch statement instead of the three if statements:

  switch (e.getActionCommand()) {
  case "Start":
      run.setEnabled(false);
      ...
      break;
  case "Pause":
      ...
      break;
  case "Reset":
      ...
      break;
  }
  

• In NewBoard.neighbours, you wrote if (x == 0 && y == 0) ; else if .... Never do that. This semicolon looks like a bug to everyone. You should rather use empty braces {} or just invert the condition, so that it reads if (x != 0 || y != 0) ....

• In NewBoard.neighbours, the condition anything == true is always the same as anything. Therefore you can leave out the == true.

• In NewBoard.setCellStatus, the parameter newCell is never used. It should be a local variable instead.

• The method setCellStatus looks very complicated. It should look like this:

  if (cell.isSelected()) 
      return neighbours == 2 || neighbours == 3;
  else 
      return neighbours == 3;
  

Many of the above improvements (and literally thousands more) are automatically suggested by the IntelliJ IDE, so you should try that. Then you get these nice hints for free without asking any human.