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

Number Memory Game (JApplet)

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

Problem

I coded a JApplet game that allows the user to play a game that displays a sequence of 4 numbers that must then be input into a JTextField and if correct, displays a new sequence with one extra number, and continues on until the sequence is gotten wrong. It also allows the user to choose from one of 4 difficulty levels that change the amount of time between each number being displayed to the user. I have the sequence been displayed to the status for testing purposes. It also saves a high-score for each difficulty level.

Are there any problems that can be seen with how it's programmed or how I went about things? I know of one; that I shouldn't have used more than one Thread in Swing (as I did with my ScheduledExecuterService).

```
/*
*Java Version: 1.8.0_25
*Author: Peadar Ó Duinnín
*Class: COM3-A
*Student Number: R00095488
*Email Address: peter.dineen@cit.ie
*/
package Assignment1;

import java.awt.BorderLayout;
import java.awt.CardLayout;
import java.awt.Choice;
import java.awt.Color;
import java.awt.Font;
import java.awt.GridLayout;
import java.awt.Insets;
import java.awt.event.ActionEvent;
import java.awt.event.ItemEvent;
import java.awt.font.TextAttribute;
import java.util.Map;
import java.util.Random;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import java.util.concurrent.atomic.AtomicInteger;
import javax.swing.JApplet;
import javax.swing.JButton;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.JTextField;
import javax.swing.SwingConstants;

public class AUIApplet extends JApplet {
private final int APPLET_WIDTH = 600;
private final int APPLET_HEIGHT = 400;
private final int BASE_AMOUNT = 4; //base amount of numbers displayed in game
private final int[] DIFF_TIMES = {2000, 1400, 1000, 600}; //time numbers are displayed in each

Solution

Code smell:


Large class: a class that has grown too large. See God object.

You have put the whole game, all it's screens, all the game functionality, all components, everything! into one single class. This has made it very hard to see which things belongs where. For example, if I look at the method getNumbersAsString or getInsets alone, I have no idea in which part of the game they are used at. If you would have properly divided your code into a MenuScreen class, a GameScreen, and a MainApplet entry-class, that would help a lot. (It can probably be divided into even more than that as well).

At the moment, your AUIApplet class is just... too large.

Let's say that you would want to add a new difficulty to your game. Where would you change your code to do that?

private final int[] DIFF_TIMES = {2000, 1400, 1000, 600};

private final int[] HIGH_SCORES = {0, 0, 0, 0}; //stores highscores for each difficulty

difficulty.add("Easy");

difficulty.addItemListener((ItemEvent e) -> {
    switch(difficulty.getSelectedIndex()) {
        ...
        case 1:


At least four places. What if you forgot to change your code on one of them? OOOPS! - ArrayIndexOutOfBoundsException.

First of all, notice that all the cases in your big switch(difficulty.getSelectedIndex()) is actually doing the exact same thing?

difficulty.addItemListener((ItemEvent e) -> {
    difficultyLab.setText("High Score for " + difficulty.getSelectedItem() + " Difficulty:");
    difficultyScoreLab.setText(Integer.toString(HIGH_SCORES[difficulty.getSelectedIndex()]));
});


Now that alone got rid of one place where you would need to change/add code.

The biggest improvement would be to add an enum for your difficulties:

public enum Difficulty {
    EASY("Easy", 2000),
    NORMAL("Normal", 1400),
    HARD("Hard", 1000),
    EXTRA_HARD("Extra Hard", 600);

    public final String name;
    public final int diffTime;

    private Difficulty(String name, int diffTime) {
        this.name = name;
        this.diffTime = diffTime;
    }
}


Now bring out the loops and let's use this enum!

for (Difficulty diff : Difficulty.values()) {
    difficulty.add(diff.name);
}

private final int[] HIGH_SCORES = new int[Difficulty.values().length];

difficulty.select(Difficulty.NORMAL.ordinal());


Then use this method:

private Difficulty getDifficulty() {
    return Difficulty.values()[difficulty.getSelectedIndex()];
}


And you can change things like this:

int speed = DIFF_TIMES[difficulty.getSelectedIndex()];


to:

int speed = getDifficulty().diffTime;


It's nice to see that you're using Java 8, but this:

startGameBut.addActionListener((ActionEvent e) -> {
    startGame(); //to new game
});


can be written as:

startGameBut.addActionListener(e -> startGame()); //to new game


GameStatusEnum can be named simply GameStatus.

It's great that you're putting comments before your methods to describe what they're doing, but it would be even better (a lot better, actually) if you used JavaDoc

The method updateScores can be simplified (in my opinion) to:

private void updateScores(boolean isCorrect) {
    currentScore = isCorrect ? currentScore + 1 : 0;
}


Many of your methods and variables are self-explanatory, I think you are using an overly-excessive amount of comments.

Code Snippets

private final int[] DIFF_TIMES = {2000, 1400, 1000, 600};

private final int[] HIGH_SCORES = {0, 0, 0, 0}; //stores highscores for each difficulty

difficulty.add("Easy");

difficulty.addItemListener((ItemEvent e) -> {
    switch(difficulty.getSelectedIndex()) {
        ...
        case 1:
difficulty.addItemListener((ItemEvent e) -> {
    difficultyLab.setText("High Score for " + difficulty.getSelectedItem() + " Difficulty:");
    difficultyScoreLab.setText(Integer.toString(HIGH_SCORES[difficulty.getSelectedIndex()]));
});
public enum Difficulty {
    EASY("Easy", 2000),
    NORMAL("Normal", 1400),
    HARD("Hard", 1000),
    EXTRA_HARD("Extra Hard", 600);

    public final String name;
    public final int diffTime;

    private Difficulty(String name, int diffTime) {
        this.name = name;
        this.diffTime = diffTime;
    }
}
for (Difficulty diff : Difficulty.values()) {
    difficulty.add(diff.name);
}

private final int[] HIGH_SCORES = new int[Difficulty.values().length];

difficulty.select(Difficulty.NORMAL.ordinal());
private Difficulty getDifficulty() {
    return Difficulty.values()[difficulty.getSelectedIndex()];
}

Context

StackExchange Code Review Q#69908, answer score: 10

Revisions (0)

No revisions yet.