8
\$\begingroup\$

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:     [email protected]
*/
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 difficulty in milliseconds
    private final ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(0); //scheduled executer service to display numbers
    private final int INITIAL_DELAY = 1000; //initial delay before numbers begin to display

    private enum GameStatusEnum {RUNNING, WAITING, STOPPED};
    private GameStatusEnum gameStatus = GameStatusEnum.STOPPED; //enum to use in cases where the game stops prematurely (as it does when a user quits a game)
    private final int[] HIGH_SCORES = {0, 0, 0, 0}; //stores highscores for each difficulty
    private int[] currentNumbers; //stores current set of numbers being displayed to the user
    private int currentScore = 0; //stores current score of the user
    private ScheduledFuture<?> setNumberService; //scheduler for setting numbers
    private ScheduledFuture<?> endNumberService; //scheduler for re-enabling textfield and clearing number off screen after setNumberService ends

    Color titleColour = new Color(0, 51, 102); //used for title on first screen
    Font normalFont = new Font("Cambria", Font.PLAIN, 18); //used for most text
    Font titleFont = new Font("Cambria", Font.BOLD, 24); //used for title on first page
    Font headingFont = new Font("Cambria", Font.ITALIC + Font.BOLD, 20); //used for headings
    Font numberFont = new Font("Cambria", Font.BOLD, 20); //used for the numbers being displayed
    Font statusFont = new Font("Cambria", Font.BOLD, 24); //used for "Correct!", "Incorrect!", and "High Score!" notifications
    Map titleAttributes = titleFont.getAttributes(); //used for underline on title
    JPanel contPan; //main cardlayout container
    JPanel startPan, setupPan, gamePan; //main screens
    UsefulPanel usefulPan; //used to demo features not implemented in program
    JPanel rulePan, startInfoPan; //panels used in start screen "startPan"
    JPanel setupInfoPan, setupMidPan, gameOptPan, userInputPan; //panels used for setup screen "setupPan"
    JPanel scorePan, gameInfoPan, innerGamePan; //panels used for game screen "gamePan"
    JButton setupGameBut, cancelBut, endGameBut, startGameBut, checkAnswerBut, retryBut;
    JLabel highScoreLab, currentScoreLab, titleLab, difficultyLab, difficultyScoreLab, currentNumberLab, currentDiffLab, statusLab;
    JTextField answerField; //answer field
    Choice difficulty; //difficulty Choice()
    CardLayout cl = new CardLayout();

    @Override
    public void init() {
        //set applet size
        setSize(APPLET_WIDTH, APPLET_HEIGHT);

        //start screen
        startPan = new JPanel();
        startPan.setLayout(new BorderLayout(5,5));

        //setup screen
        setupPan = new JPanel();
        setupPan.setLayout(new BorderLayout(5, 5));
        setupGameBut = new JButton("New Game");
        cancelBut = new JButton("Cancel");

        //game screen
        gamePan = new JPanel();
        gamePan.setLayout(new BorderLayout(5, 5));
        endGameBut = new JButton("Quit Game");

        //heading label
        titleLab = new JLabel("Number Memory");
        titleLab.setForeground(titleColour);
        titleAttributes.put(TextAttribute.UNDERLINE, TextAttribute.UNDERLINE_ON);
        titleLab.setFont(titleFont.deriveFont(titleAttributes));

        //rule panel
        rulePan = new JPanel();
        rulePan.setLayout(new GridLayout(8, 1, 5, 5));
        rulePan.add(new JLabel("Rules:")).setFont(headingFont);
        rulePan.add(new JLabel("1. You will be shown a series of numbers, one at a time."));
        rulePan.add(new JLabel("2. You must recite the series of numbers after the last number has been displayed."));
        rulePan.add(new JLabel("3. After each correct recitation of the sequence, another sequence will play with one extra number."));
        rulePan.add(new JLabel("Note: You can decrease/increase the time each number displays for by changing the difficulty."));

        //difficulty selection Choice()
        difficulty = new Choice();
        difficulty.add("Easy");
        difficulty.add("Normal");
        difficulty.add("Hard");
        difficulty.add("Extra Hard");
        difficulty.select(1);

        //game option panel
        gameOptPan = new JPanel();
        gameOptPan.setLayout(new GridLayout(1, 2, 5, 5));
        startGameBut = new JButton("Start Game");
        gameOptPan.add(startGameBut);
        gameOptPan.add(difficulty);

        //start info panel
        startInfoPan = new JPanel();
        startInfoPan.setLayout(new BorderLayout(5, 5));
        startInfoPan.add(rulePan, BorderLayout.CENTER);

        //setup high score display panel for on-selection display of high scores for 
        //each difficulty level
        setupMidPan = new JPanel();
        setupMidPan.setLayout(new GridLayout(1, 8, 5, 5));
        difficultyLab = new JLabel();
        difficultyLab.setText("High Score for " + difficulty.getSelectedItem() + " Difficulty:");
        difficultyScoreLab = new JLabel(Integer.toString(HIGH_SCORES[difficulty.getSelectedIndex()]));
        setupMidPan.add(difficultyLab);
        setupMidPan.add(difficultyScoreLab);

        //setup info panel
        setupInfoPan = new JPanel();
        setupInfoPan.setLayout(new BorderLayout(5, 5));
        setupInfoPan.add(gameOptPan, BorderLayout.SOUTH);
        setupInfoPan.add(setupMidPan, BorderLayout.CENTER);

        //user input panel
        userInputPan = new JPanel();
        userInputPan.setLayout(new GridLayout(2, 1, 5, 5));
        answerField = new JTextField(30);
        userInputPan.add(answerField);
        checkAnswerBut = new JButton("Check Answer");
        userInputPan.add(checkAnswerBut);

        //game info panel
        gameInfoPan = new JPanel();
        gameInfoPan.setLayout(new GridLayout(4, 1, 5, 5));
        currentNumberLab = new JLabel("", SwingConstants.CENTER);
        currentNumberLab.setFont(numberFont);
        gameInfoPan.add(currentNumberLab);
        gameInfoPan.add(userInputPan);
        gameInfoPan.add(new JLabel("Enter each number seperated by a comma. e.g. '1,2,3,11,22,33'"));
        statusLab = new JLabel("", SwingConstants.CENTER);
        statusLab.setFont(statusFont);
        gameInfoPan.add(statusLab);

        //score panel
        scorePan = new JPanel();
        scorePan.setLayout(new GridLayout(10, 1, 5, 5));
        currentDiffLab = new JLabel("");
        highScoreLab = new JLabel("High Score: " + HIGH_SCORES[difficulty.getSelectedIndex()] + "  ");
        currentScoreLab = new JLabel("Current Score: " + currentScore + "  ");
        scorePan.add(currentDiffLab);
        scorePan.add(highScoreLab);
        scorePan.add(currentScoreLab);

        //inner game panel
        innerGamePan = new JPanel();
        innerGamePan.setLayout(new BorderLayout(5, 5));
        retryBut = new JButton("Retry");
        innerGamePan.add(retryBut, BorderLayout.SOUTH);
        innerGamePan.add(scorePan, BorderLayout.EAST);
        innerGamePan.add(gameInfoPan, BorderLayout.CENTER);

        //adding to start panel
        startPan.add(titleLab, BorderLayout.NORTH);
        startPan.add(setupGameBut, BorderLayout.SOUTH);
        startPan.add(startInfoPan, BorderLayout.CENTER);

        //adding to setup panel
        setupPan.add(setupInfoPan, BorderLayout.CENTER);
        setupPan.add(cancelBut, BorderLayout.SOUTH);

        //adding to game panel
        gamePan.add(endGameBut, BorderLayout.SOUTH);
        gamePan.add(innerGamePan, BorderLayout.CENTER);

        //setting up container panel and adding each screen to it
        contPan = new JPanel();
        contPan.setLayout(cl);
        contPan.add(startPan, "Start Applet Screen");
        contPan.add(setupPan, "Setup Game Screen");
        contPan.add(gamePan, "New Game Screen");

        setupGameBut.addActionListener((ActionEvent e) -> {
            newGame(); //to setup screen
        });

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

        retryBut.addActionListener((ActionEvent e) -> {
            startGame(); //to new game from inside previously lost game
        });

        cancelBut.addActionListener((ActionEvent e) -> {
            quitGame(); //quits setup and returns to main menu
        });

        endGameBut.addActionListener((ActionEvent e) -> {
            quitGame(); //quits game
        });

        //used to update speeds that the numbers are displayed at. Each difficulty has 
        //it's own high score
        difficulty.addItemListener((ItemEvent e) -> {
            switch(difficulty.getSelectedIndex()) {
                case 0: 
                    difficultyLab.setText("High Score for " + difficulty.getSelectedItem() + " Difficulty:");
                    difficultyScoreLab.setText(Integer.toString(HIGH_SCORES[difficulty.getSelectedIndex()]));
                    break;
                case 1: 
                    difficultyLab.setText("High Score for " + difficulty.getSelectedItem() + " Difficulty:");
                    difficultyScoreLab.setText(Integer.toString(HIGH_SCORES[difficulty.getSelectedIndex()]));
                    break;
                case 2: 
                    difficultyLab.setText("High Score for " + difficulty.getSelectedItem() + " Difficulty:");
                    difficultyScoreLab.setText(Integer.toString(HIGH_SCORES[difficulty.getSelectedIndex()]));
                    break;
                case 3: 
                    difficultyLab.setText("High Score for " + difficulty.getSelectedItem() + " Difficulty:");
                    difficultyScoreLab.setText(Integer.toString(HIGH_SCORES[difficulty.getSelectedIndex()]));
                    break;
            }
        });

        answerField.addActionListener((ActionEvent e) -> {
           checkAnswer(answerField.getText()); //submits answer
        });

        checkAnswerBut.addActionListener((ActionEvent e) -> {
           checkAnswer(answerField.getText()); //submits answer
        });

        //add container panel
        this.add(contPan);
    }

    //brings user to game setup screen
    private void newGame() {
        difficultyLab.setText("High Score for " + difficulty.getSelectedItem() + " Difficulty:");
        difficultyScoreLab.setText(Integer.toString(HIGH_SCORES[difficulty.getSelectedIndex()]));
        statusLab.setText("");
        cl.show(contPan, "Setup Game Screen");
    }

    //starts a new game
    private void startGame() {
        answerField.setText("");
        cl.show(contPan, "New Game Screen");
        currentNumbers = getRandomNumbers(currentScore);
        gameStatus = GameStatusEnum.RUNNING;
        retryBut.setVisible(false);
        updateScorePan(currentScore);
        printNumbers(currentNumbers);
    }

    //returns an array of random numbers between 1 and 99 inclusive for the sum of the base 
    //amount (4) and the user's current score
    private int[] getRandomNumbers(int currentScore) {
        int[] randomNums = new int[BASE_AMOUNT + currentScore];

        for (int i = 0; i < randomNums.length; i++) {
            randomNums[i] = getRandomNumber();
        }
        return randomNums;
    }

    //return singular random number
    private int getRandomNumber() {
        Random rand = new Random();
        int max = 99;
        int min = 0;
        int randomNum = rand.nextInt((max-min) + 1) + min;
        return randomNum;
    }

    //deals with setting up for the numbers being displayed and calling scheduleNumbers()
    //which executes schedulers for display of numbers
    private void printNumbers(int[] randomNumbers) {
        int speed = DIFF_TIMES[difficulty.getSelectedIndex()];
        int amount = BASE_AMOUNT + currentScore;
        answerField.setEditable(false); //stops user from entering text before the game has started
        checkAnswerBut.setEnabled(false); //stops user from submitting an answer when the program isn't ready
        showStatus(getNumbersAsString());
        scheduleNumbers(randomNumbers, speed, amount);
    }

    //executes schedulers for display of numbers
    public void scheduleNumbers(int[] randomNumbers, int speed, int amount) {
        long initialDelay = INITIAL_DELAY;
        final AtomicInteger curNumber = new AtomicInteger(-1); //used so can be updated in Runnable setNumber() lambda
        final Runnable setNumber = () -> {
            currentNumberLab.setText(Integer.toString(randomNumbers[curNumber.incrementAndGet()])); //sets text to next number
        };
        setNumberService = scheduler.scheduleAtFixedRate(setNumber, initialDelay, speed, MILLISECONDS); //schedules calls of setNumber() at specific intervals for certain time
        //scheduled to clean up after setNumberService by enabling components, removing number display and canceling setNumberService
        endNumberService = scheduler.schedule(() -> {
            currentNumberLab.setText("");
            answerField.setEditable(true);
            checkAnswerBut.setEnabled(true);
            gameStatus = GameStatusEnum.WAITING;
            setNumberService.cancel(true);
        }, ((speed * amount)+ initialDelay), MILLISECONDS);
    }

    //updates in-game score panel 
    private void updateScorePan(int curScore) {
        currentDiffLab.setText(difficulty.getSelectedItem());
        highScoreLab.setText("High Score: " + HIGH_SCORES[difficulty.getSelectedIndex()] + "  ");
        currentScoreLab.setText("Current Score: " + curScore + "  ");
    }

    //ends game
    private void quitGame() {
        cl.show(contPan, "Start Applet Screen");
        reset();
    }

    //performs actions usually taken by endNumberService in the case of a premature exit of game
    private void reset() {
        currentNumberLab.setText("");
        currentScore = 0;
        if (gameStatus == GameStatusEnum.RUNNING) {
            setNumberService.cancel(true); //cancel setNumberService
            endNumberService.cancel(true); //cancel endNumberService
        }
        gameStatus = GameStatusEnum.STOPPED;
    }

    //checks users answer against string of random numbers displayed
    private void checkAnswer(String answer) {
        String numbersAsString = getNumbersAsString();
        if (answer.equals(numbersAsString)) {
            answerCorrect();
        }
        else {
            answerIncorrect();
        }
    }

    //returns numbers displayed as string
    private String getNumbersAsString() {
        String numbersAsString = "";
        for (int number : currentNumbers) {
            numbersAsString += number + ",";
        }
        numbersAsString = numbersAsString.substring(0, numbersAsString.length()-1);
        return numbersAsString;
    }

    //updates current score in the case of correct answer
    private void updateScores(boolean isCorrect) {
        if (isCorrect) {
            currentScore++;
        }
        else {
            currentScore = 0;
        }
    }

    //updates high score in the case of it being beaten
    private boolean checkHighScore() {
        int highScore = HIGH_SCORES[difficulty.getSelectedIndex()];
        if (currentScore > highScore) {
            HIGH_SCORES[difficulty.getSelectedIndex()] = currentScore;
            return true;
        }
        return false;
    }

    //if answer is correct
    private void answerCorrect() {
        System.out.println("Correct!");
        updateScores(true);
        if(checkHighScore()){
            statusLab.setText("New High Score!");
        }
        else {
            statusLab.setText("Correct!");
        }
        startGame(); //starts a new round
    }

    //if answer incorrect
    private void answerIncorrect() {
        statusLab.setText("Incorrect!");
        updateScores(false);
        retryBut.setVisible(true); //displays button which allows user to retry without returning to setup
    }

    //adds insets for aesthetic reasons around contPan
    @Override
    public Insets getInsets() {
        return new Insets(10, 10, 10, 10);
    }
}
\$\endgroup\$

1 Answer 1

10
\$\begingroup\$

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.

\$\endgroup\$
2
  • \$\begingroup\$ Amazing answer, thank you! I've been reading Code Complete by Steve McConnell so I noticed myself doing some of these (like shoving all the code in one class). But as it was a submission for college (hence the excessive amount of comments); I didn't want to get bogged down by changing a lot of the fundamentals of my program and finding myself not being able to fix it in time for submission. This helps me understand how to approach it, though. And the other suggestions are greatly appreciated. :) \$\endgroup\$
    – DagdA
    Commented Dec 15, 2014 at 15:02
  • 1
    \$\begingroup\$ @peadar I hope you will post a follow-up question later, would be nice to see the improvements! \$\endgroup\$ Commented Dec 15, 2014 at 17:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.