patternjavaModerate
Number Memory Game (JApplet)
Viewed 0 times
gamenumberjappletmemory
Problem
I coded a
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
```
/*
*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
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
At the moment, your
Let's say that you would want to add a new difficulty to your game. Where would you change your code to do that?
At least four places. What if you forgot to change your code on one of them? OOOPS! -
First of all, notice that all the cases in your big
Now that alone got rid of one place where you would need to change/add code.
The biggest improvement would be to add an
Now bring out the loops and let's use this enum!
Then use this method:
And you can change things like this:
to:
It's nice to see that you're using Java 8, but this:
can be written as:
GameStatusEnum can be named simply
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
Many of your methods and variables are self-explanatory, I think you are using an overly-excessive amount of comments.
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 gameGameStatusEnum 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.