2
\$\begingroup\$

I have the following code in my class and its init() method. It works fine as it is but I'm wondering if there's a better way to do this so that my init() and initialisations aren't so cluttered.

Should I have separate initialisation methods for each component that my init() can call? I'm mostly trying to just learn good practices for GUIs and events. I left a link for the rest of my program below the code excerpt.

private final int APPLET_WIDTH = 600;
private final int APPLET_HEIGHT = 400;

private int[] highScores = {0, 0, 0, 0};
private int[] currentNumbers;
private String numsDisplayed;
private int currentScore = 0;

final Font normalFont = new Font("Cambria", Font.PLAIN, 18);
final Font headingFont = new Font("Cambria", Font.BOLD, 24);
JPanel contPan, startPan, setupPan, gamePan;
JPanel scorePan, startInfoPan, rulePan, setupInfoPan, setupMidPan, gameOptPan, gameInfoPan, userInputPan;
JButton setupGameBut, cancelBut, endGameBut, startGameBut, checkAnswerBut;
JLabel highScoreLab, currentScoreLab, headingLab, difficultyLab, difficultyScoreLab, currentNumberLab;
JTextField answerField;
Choice difficulty;
CardLayout cl = new CardLayout();
Image waitingPic, correctPic, incorrectPic, highScorePic;

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

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

    //setup screen
    setupPan = new JPanel();
    setupPan.setLayout(new BorderLayout());

    setupGameBut = new JButton("New Game");
    cancelBut = new JButton("Cancel");

    //game screen
    gamePan = new JPanel();
    gamePan.setLayout(new BorderLayout());

    endGameBut = new JButton("Quit Game");

    //heading label
    headingLab = new JLabel("Number Memory");
    headingLab.setFont(headingFont);

    //rule panel
    rulePan = new JPanel();
    rulePan.setLayout(new GridLayout(8,1));
    rulePan.add(new JLabel("Rules:"));
    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
    difficulty = new Choice();
    difficulty.add("Easy");
    difficulty.add("Normal");
    difficulty.add("Hard");
    difficulty.add("Extra Hard");
    difficulty.select(1);

    difficultyScoreLab = new JLabel("" + highScores[difficulty.getSelectedIndex()] + "");

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

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

    //set up high score display panel
    setupMidPan = new JPanel();
    setupMidPan.setLayout(new GridLayout(1,8));
    difficultyLab = new JLabel();
    difficultyLab.setText("High Score for " + difficulty.getSelectedItem() + " Difficulty:");
    setupMidPan.add(difficultyLab);
    setupMidPan.add(difficultyScoreLab);

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

    //user input panel
    userInputPan = new JPanel();
    userInputPan.setLayout(new GridLayout(2,1));
    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(5,1));
    currentNumberLab = new JLabel("");
    gameInfoPan.add(currentNumberLab);
    gameInfoPan.add(userInputPan);
    gameInfoPan.add(new JLabel("Enter each number followed by a space."));

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

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

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

    //adding to game panel
    gamePan.add(endGameBut, BorderLayout.SOUTH);
    gamePan.add(headingLab, BorderLayout.NORTH);
    gamePan.add(gameInfoPan, BorderLayout.CENTER);
    gamePan.add(scorePan, BorderLayout.EAST);

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

    //action listeners
    setupGameBut.addActionListener((ActionEvent e) -> {
        newGame();
    });

    startGameBut.addActionListener((ActionEvent e) -> {
        //moreNumbers();
        startGame(0);
    });

    cancelBut.addActionListener((ActionEvent e) -> {
        quitGame();
    });

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

    difficulty.addItemListener((ItemEvent e) -> {
        switch(difficulty.getSelectedIndex()) {
            case 0: 
                difficultyLab.setText("High Score for " + difficulty.getSelectedItem() + " Difficulty:");
                difficultyScoreLab.setText(Integer.toString(highScores[difficulty.getSelectedIndex()]));
                break;
            case 1: 
                difficultyLab.setText("High Score for " + difficulty.getSelectedItem() + " Difficulty:");
                difficultyScoreLab.setText(Integer.toString(highScores[difficulty.getSelectedIndex()]));
                break;
            case 2: 
                difficultyLab.setText("High Score for " + difficulty.getSelectedItem() + " Difficulty:");
                difficultyScoreLab.setText(Integer.toString(highScores[difficulty.getSelectedIndex()]));
                break;
            case 3: 
                difficultyLab.setText("High Score for " + difficulty.getSelectedItem() + " Difficulty:");
                difficultyScoreLab.setText(Integer.toString(highScores[difficulty.getSelectedIndex()]));
                break;
        }
    });

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

Full code can be found here. A good bit of it is completely wrong as I'm trying to wrap my head around threads and other stuff. I've commented those parts out so that it works. Don't use the NumberThread class. It's not working, but I left it in just in-case.

\$\endgroup\$
0
5
\$\begingroup\$

I'm not a UI developer, so I'm afraid I can't offer too much advice, except from a basic Java perspective.

  • Your applet width and height constants should be passed in via the constructor.

  • All the fields after currentScore are set to package scope. Is there a reason why they're not public?

  • For these same fields, it's a code smell to me to see so many variables, but then again, I'm not sure if this is the norm for swing apps. I haven't looked at the full code, but my initial reaction to this would be to have a single map of Strings or Enums to JComponent or whatever all the Swing objects extend which stores them. I'll expand on this later.

  • For your init method, you're doing a lot of independent operations which you can extract into individual methods to increase the abstraction layer of the initialization. So instead of thinking, I'm creating panel Foo with with labels X, Y, and Z, you are just creating your rule panel. You can pass in an previous Swing component as necessary. The benefit of this is that your initialization code then becomes something like:

    createX()
    createY()
    createZ()
    //etc
    
  • Back to what I was saying about a map of JComponents, suppose you had an EnumMap<UI, JComponent>, where UI is a crappy name for an enum of each of the two dozen Swing fields you have. Then you can do something like:

    private static enum UI { START, SETUP, GAME, .... };
    
    private final EnumMap<UI, JComponent> components = new EnumMap<UI, JComponent>(UI.class);
    
    public void init(){
      createStartScreen(components);
      ...
    }
    
    private void createStartScreen(EnumMap<UI, JComponent> components) {
        startPan = new JPanel();
        startPan.setLayout(new BorderLayout());
        components.put(UI.START, startPan);
    }
    
  • For the action listeners near the end, I believe you can replace (ActionEvent e) with just (e). There's probably an even nicer way of dealing with these, but I haven't used Java 8 too much by now to know.

  • Is your final switch statement actually doing anything? I don't see a difference between any of the cases, and in case I'm missing it, the code duplication makes me want to see that as another extracted function.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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