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

How to split this Bingo game into 3 separate classes?

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

Problem

I have a single file of a java Bingo program that I want to split into three separate classes (BingoMain, BingoGUI, BingoCard). Extending a program across several files is something I have never really done, so a little help is appreciated. I also want to add the ability to extend the current program to include functionality for up to 5 players. I know that all the code I need is pretty much in here already. Which lines/parts would go in which class and what would I edit in order to add the desired functionality? I don't expect any code, just some directions to get me on my way.

```
import java.awt.Container;
import java.awt.Font;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import javax.swing.*;

public class Bingo extends JFrame {

public static void main (String[] args) {

Integer[][] bingoCard = new Integer[5][5];
boolean[][] called = new boolean[][] { {false,false,false,false,false},
{false,false,false,false,false},
{false,false,true ,false,false},
{false,false,false,false,false},
{false,false,false,false,false}};
JFrame myBingoGUI;
/*
* Fill an array with 15 numbers (1-15) for the "B" column, put the numbers in random order, and
* put the first 5 of those numbers in the "B" column of the bingo card. Continue with I,N,G,and O columns.
*/
Integer[] row = new Integer[15];

for (int i=0; i");
else if (called[i][j]) System.out.print("");
else
System.out.print(" "+bcard[i][j]+" ");
if (bcard[i][j]");
else if (called[i][j]) returnString+= ("");
else
returnString+= (" "+bcard[i][j]+" ");
if (bcard[i][j]=0;i--)

Solution

Review of Existing Code

  • Not object oriented: As you probably already realize, your code is written in a procedural style, not object-oriented, which is the norm in Java. You can tell because every function is static. Note that although you declared public class Bingo extends JFrame, it runs just the same if you remove extends JFrame. What's actually happening is that your myBingoGUI is the JFrame.



  • Prefer unboxed types: Unless you have a good reason to use boxed types (Integer), you should use the unboxed primitive instead (int). Boxed types make your code clumsy and incur unnecessary overhead. Although you do need to use an ArrayList to take advantage of Collections.shuffle() while randomizing the board, I would still store the board using an unboxed int[][].



-
Initialization of called: This takes less code:

boolean[][] called = new boolean[5][5];
called[5/2][5/2] = true;


  • Helper functions should be private: Some of your functions are obviously intended for internal use only — for example, fillRow(), randomize(), putRowInBingoCard(). You would never want any other code to call putRowInBingoCard(), so mark it private instead of public. That goes for randomize() too — even though the function has some conceivable general utility, you wouldn't want to let a hypothetical Blackjack game use Bingo.randomize(), so don't expose it.



  • Misnomer: fillRow() and putRowInBingoCard() actually work on a column, not a row.



  • Dead code: printBingoCard() is never used. Likewise with printArray(), as you noted.



-
Simplification: The code to assign calledColumn and calledValue is unnecessarily verbose. The following is equivalent:

int calledColumn = "BINGO".indexOf(Character.toUpperCase(calledNumber.charAt(0)));
int calledValue = Integer.parseInt(calledNumber.substring(1));


-
Error handling: If you input an empty string, it crashes with a StringIndexOutOfBoundsException. If you give it just a number, it crashes with an ArrayIndexOutOfBoundsException. If you hit "Cancel" in the input dialog, it crashes with a NullPointerException. (I would expect it to exit gracefully — call dispose() on the frame and break from the loop.)

  • Use a StringBuilder: For repeated concatenation, use a StringBuilder for efficiency.



  • Naming convention for predicates: In Java, a convention is to name a method isSomething() if the method takes no arguments, performs a test with no side effects, and returns a boolean. Therefore, you should rename winner() to isWinner().



OOP Refactoring: First step

Notice that you pass the arrays bingoCard and called nearly everywhere. That is the state that should be stored as instance variables in a BingoCard object. I'll provide an outline of a BingoCard class for you to fill in:

public class BingoCard {
    private int[][] numbers;
    private boolean[][] called;

    public BingoCard() {
        numbers = new int[5][5];
        called = new boolean[5][5];
        called[5/2][5/2] = true;

        // Fill in numbers randomly as appropriate
        ...
    }

    public String toString() {
        ...
    }

    public boolean isWinner() {
        ...
    }

    public void mark(String calledNumber) throws IllegalArgumentException {
        ...
    }

    // Let's keep this here for now...
    public static void main (String[] args) {
        BingoCard card = new BingoCard();

        JFrame myBingoGUI;
        myBingoGUI=new JFrame();
        myBingoGUI.setSize(250, 250);
        myBingoGUI.setLocation(400, 400);
        myBingoGUI.setTitle("BINGO");
        myBingoGUI.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        myBingoGUI.setVisible(true);
        Container myContentPane = myBingoGUI.getContentPane();
        JTextArea myTextArea = new JTextArea();
        myTextArea.setFont(new Font("Monospaced", Font.PLAIN,14));
        myContentPane.add(myTextArea);

        myTextArea.setText(card.toString());
        System.out.println(card);

        // Play the game:   
        String error = "";
        String calledNumber = null;
        while (!card.isWinner() &&
               null != (calledNumber = JOptionPane.showInputDialog(null, error + "Enter a BINGO call:")) {
            try {
                card.mark(calledNumber);
                error = "";
            } catch (IllegalArgumentException badCall) {
                error = "Invalid input: " + badCall.getMessage() + "\n";
            }

            // Print the updated BINGO card.
            myTextArea.setText(card.toString());
            System.out.println(card);

       }
       if (card.isWinner()) {
           myTextArea.append("\n\nBINGO!!");
       }
       if (null == calledNumber) {
           myBingoUI.dispose();
       }
    }
}


If you need to, you can add helper methods. Remember, however, such helper methods should not be part of the class's interface, and should therefore be marked private. (Pe

Code Snippets

boolean[][] called = new boolean[5][5];
called[5/2][5/2] = true;
int calledColumn = "BINGO".indexOf(Character.toUpperCase(calledNumber.charAt(0)));
int calledValue = Integer.parseInt(calledNumber.substring(1));
public class BingoCard {
    private int[][] numbers;
    private boolean[][] called;

    public BingoCard() {
        numbers = new int[5][5];
        called = new boolean[5][5];
        called[5/2][5/2] = true;

        // Fill in numbers randomly as appropriate
        ...
    }

    public String toString() {
        ...
    }

    public boolean isWinner() {
        ...
    }

    public void mark(String calledNumber) throws IllegalArgumentException {
        ...
    }

    // Let's keep this here for now...
    public static void main (String[] args) {
        BingoCard card = new BingoCard();

        JFrame myBingoGUI;
        myBingoGUI=new JFrame();
        myBingoGUI.setSize(250, 250);
        myBingoGUI.setLocation(400, 400);
        myBingoGUI.setTitle("BINGO");
        myBingoGUI.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        myBingoGUI.setVisible(true);
        Container myContentPane = myBingoGUI.getContentPane();
        JTextArea myTextArea = new JTextArea();
        myTextArea.setFont(new Font("Monospaced", Font.PLAIN,14));
        myContentPane.add(myTextArea);

        myTextArea.setText(card.toString());
        System.out.println(card);

        // Play the game:   
        String error = "";
        String calledNumber = null;
        while (!card.isWinner() &&
               null != (calledNumber = JOptionPane.showInputDialog(null, error + "Enter a BINGO call:")) {
            try {
                card.mark(calledNumber);
                error = "";
            } catch (IllegalArgumentException badCall) {
                error = "Invalid input: " + badCall.getMessage() + "\n";
            }

            // Print the updated BINGO card.
            myTextArea.setText(card.toString());
            System.out.println(card);

       }
       if (card.isWinner()) {
           myTextArea.append("\n\nBINGO!!");
       }
       if (null == calledNumber) {
           myBingoUI.dispose();
       }
    }
}
public class BingoCard {
    public static interface Listener {
        void handleBingoCardChanged(BingoCard card);
    }

    private HashSet<Listener> listeners = new HashSet<Listener>;

    // ... the previously developed the BingoCard code here ...

    public void addListener(Listener l) {
        this.listeners.add(l);
    }

    public void removeListener(Listener l) {
        this.listeners.remove(l);
    }

    private void fireChangeEvent() {
        for (Listener l : this.listeners) {
            l.handleBingoCardChanged(this);
        }
    }
}
public class BingoUI extends JFrame implements BingoCard.Listener {
    private JTextArea textArea;

    public BingoUI(BingoCard card) {
        // Rough outline
        this.setSize(...);
        ...
        this.textArea = new JTextArea();
        card.addChangeListener(this);
        this.textArea.setText(card.toString());
    }

    public void handleBingoCardChanged(BingoCard card) {
        this.textArea.setText(card.toString());
        if (card.isWinner()) {
            this.textArea.append("\n\nBINGO!!");
        }
    }
}

Context

StackExchange Code Review Q#35131, answer score: 8

Revisions (0)

No revisions yet.