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

• Roland Illig: It's a nice question for review. I like especially that you gave a one-line description of each class. That's almost too good for a beginner. :)
• NGH: Do I sense sarcasm? ;) I can elaborate on my brief descriptions if needs be :)
• Roland Illig: No, no sarcasm. I really meant it, since most other people don't give these comments, but still I find them very useful.
• NGH: Oh well thank you very much sir! I'll make sure I continue to do this :)
• NGH: Would really appreciate a review of the actual code mind you haha

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.

• NGH: Thank you kindly sir, in your opinion does it meet the principals of OO programming on the whole or? There are some parts of your response I don't agree with but overall I will take the advice on board