Number-guessing game in Java with Swing UI

Question

I am trying not to pick up any bad habits while improving the way I write programs. Any suggestions on the following lines of code?

package guessGame;

import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.Random;

// Main Class
public class GuessGame extends JFrame {

// Declare class variables
private static final long serialVersionUID = 1L;
public static Object prompt1;
private JTextField userInput;
private JLabel comment = new JLabel(" ");
private JLabel comment2 = new JLabel(" ");
private int randomNumber;
private int counter = 0;

// Constructor
public GuessGame() {
    super("Guessing Game");
    setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

    // Content pane
    setLayout(new FlowLayout());
    Container c = getContentPane();

    // Create components
    JButton guessButton = new JButton("Try the number");
    JButton newGameButton = new JButton("New Game");
    JButton quitButton = new JButton("Quit");
    JLabel prompt1 = new JLabel("I have a number between 1 and 1000.");
    JLabel prompt2 = new JLabel("Can you guess the number?");
    JLabel prompt3 = new JLabel("Please enter your guess: ");
    comment = new JLabel(" ");
    comment2 = new JLabel(" ");
    userInput = new JTextField(5);

    // Adding components to the pane
    c.add(prompt1);
    c.add(prompt2);
    c.add(prompt3);
    c.add(userInput);
    c.add(guessButton);
    c.add(newGameButton);
    c.add(quitButton);
    c.add(comment);
    c.add(comment2);

    // Set the mnemonic
    guessButton.setMnemonic('T');
    newGameButton.setMnemonic('N');
    quitButton.setMnemonic('Q');

    // Format pane
    setSize(300, 200);
    setLocationRelativeTo(null);
    setVisible(true);
    setResizable(false);

    initializeNumber();

    // Create the button handlers
    GuessButtonHandler ghandler = new GuessButtonHandler(); // instantiate
                                                            // new object
    guessButton.addActionListener(ghandler); // add event listener

    NewButtonHandler nhandler = new NewButtonHandler();
    newGameButton.addActionListener(nhandler);

    QuitButtonHandler qhandler = new QuitButtonHandler();
    quitButton.addActionListener(qhandler);
} // End constructor

// 
private void initializeNumber() {
    randomNumber = new Random().nextInt(1000) + 1;
}

// GuessButton inner class
private class GuessButtonHandler implements ActionListener {
    public void actionPerformed(ActionEvent e) {

        // Declare class variables
        int getUserInput;
        int diff;
        int Difference;

        // Validate input and if statements for user input
        try {
            getUserInput = Integer.parseInt(userInput.getText().trim());
            counter++;

            if (getUserInput == randomNumber) {
                JOptionPane.showMessageDialog(null, "Correct! It took you "
                        + counter + " guesses", "Random Number: "
                        + randomNumber, JOptionPane.INFORMATION_MESSAGE);
                initializeNumber();
                return;
            }
            if (getUserInput > randomNumber) {
                comment2
                        .setText("The guess was too HIGH. Try a lower number.");
                comment2.setForeground(Color.WHITE);
                diff = getUserInput - randomNumber;
                Difference = Math.abs(diff);
            } else {
                comment2
                        .setText("The guess was too LOW. Try a higher number.");
                comment2.setForeground(Color.WHITE);
                diff = randomNumber - getUserInput;
                Difference = Math.abs(diff);
            }

            if (Difference >= 30) {
                comment.setText("Your Cold. ");
                comment.setForeground(Color.WHITE);
                GuessGame.this.setBackgroundColor(Color.BLUE);
            }

            if (Difference <= 15) {
                comment.setText("Your getting Warm");
                comment.setForeground(Color.WHITE);
                GuessGame.this.setBackgroundColor(Color.RED);
            }
        } catch (NumberFormatException ex) {
            comment.setText("Enter a VALID number!");
        }
    }
} // End GuessButtonHandler

// NewButton inner class
private class NewButtonHandler implements ActionListener {
    public void actionPerformed(ActionEvent e) {
        GuessGame app = new GuessGame();

    }
} // End NewButtonHandler

// QuitButton inner class
private class QuitButtonHandler implements ActionListener {
    public void actionPerformed(ActionEvent e) {
        System.exit(0);
    }
} // End QuitButtonHandler

// Setting background color
private void setBackgroundColor(Color RED) {
    getContentPane().setBackground(RED);
}

// Main method
public static void main(String args[]) {
    // instantiate GuessGame object
    GuessGame app = new GuessGame();

}// End main method
}// End main class

you can minimize that long if-else block by having a separate method — Kinjal, Oct 2 '13 at 4:07

WeaponsGrade · Accepted Answer · 2013-10-02 05:19:31Z

Problems I see:

comment and comment2 are initialised twice, once in the class definition and again in the constructor.
If you click the New Game button, you'll get a new window but the old one will still be there. (see also the last point below)

You may want to provide a reset() method that will both initialise a random number and also reset the comment labels to their default values. Then you can call reset() both from your constructor and also when a game is done. Or even when a new game is asked for.
prompt1 is declared as a class member, but never used. You can get rid of it at the class level.

Try to find repeated parts of code, and think about how you can pull those out to a reusable method. Keep your code DRY.

Some pro-level tips and style-oriented things:

I usually like to make Swing components as self-contained as possible (see examples below). Therefore, for any UI-specific logic (such as internal event listening) I have the component (your JFrame) implement the listener interface (e.g., ActionListener for button clicks). If that triggers some type of application event, I'd fire off that type of event from within the actionPerformed() method of the UI component. The only time I extract an explicit separate class for Swing listeners is when they're generally useful across several UI components, which isn't real often.

This helps to isolate the UI as being Swing-based; the rest of your application receives an application-level event and it doesn't need to know if it was from a Swing button, a web button, a service invocation, or whatever. (You shouldn't be importing Swing interfaces into non-UI classes).

For trivial Swing applications like this, using inner classes to implement listeners is fine, and indeed it's a common practice. It just seems more clean (to me at least) to make a self-contained component out of it. For large Swing applications, however, all those extra classes can eat up your PermGen memory.
For convenience, the JFrame.add() method targets the content pane, so it's not strictly necessary to reference it directly. But it's good to remember that you need to deal with the content pane.

I also typically localise (small-L) the configuration of my internal components instead of spreading them out:

JButton button = new JButton("Try the number");
button.setMnemonic('T');
button.addActionListener(this);
add(button);

Now I can reuse the button variable name for the next one. Same with labels. After you create a couple of panels or frames, you'll start to notice some common coding patterns you use. Those can be extracted into static methods in a utility class. For instance, you could have a static method that creates and returns a new button given some text, a mnemonic, a command, and a listener.

Doing some of that, it might look like:

public static JButton mkButton(String text, Character mnemonic, String command, ActionListener listener) {
  final JButton button = new JButton(text);
  if (mnemonic != null) button.setMnemonic(mnemonic);
  if (command != null) button.setActionCommand(command);
  if (listener != null) button.addActionListener(listener);
  return button;
}

These utility methods really start to shine when you start using more advanced layout managers. Next, create constants for your commands in your class:

private static final String ACTION_GUESS = "guess";
private static final String ACTION_NEW = "new";
private static final String ACTION_QUIT = "quit";

then, in the constructor it's just:

add(mkButton("Try the number", 'T', ACTION_GUESS, this));

finally, you can switch on the action command of your button to either do trivial work in the switch or dispatch to a method for more complex operations:

@Override
public void actionPerformed(ActionEvent e) {
  switch(e.getActionCommand()) {
    case ACTION_GUESS:
      guess();
      break;

    case ACTION_QUIT:
      System.exit(0);
      break;

    case ACTION_NEW:
      reset();
      break;
  }
}

Having said all of the above concerning buttons, if you're building a large complex Swing app, you'll want to define javax.swing.Actions and reuse them in buttons and menus.
After you add all of your components, call pack(). That will set up dimensions if you want to tweak them (as opposed to setting absolute dimensions as you have).
You definitely don't want to setVisible(true) (or other side-effects) in a constructor. Let calling code determine when to display UI components.
Unless you know absolutely that your frame will only ever be used from main(), don't set the default close operation in the constructor; let the calling code set that (in this case, main()).

DominikM · Answer 2 · 2013-10-02 05:35:03Z

Name your variables properly

// Declare class variables
        int getUserInput;
        int diff;
        int Difference;

They are not class variables, they are declared within a method. But my point is to Stick to conventions - diff and Difference does not adhere same naming convention and also its hard to tell what is the purpose of each one, because whats the difference between diff (abbreviation of difference) and actual Difference?

This code fragment

public void actionPerformed(ActionEvent e) {
        GuessGame app = new GuessGame();

    }

does nothing. It just create new object which is lost after method is executed because is stored in local variable.

And do not put much code into constructors. Use construcotrs only for instance initialization. Not for actual logic. So you main method then would look like.

GuessGame app = new GuessGame();
app.start();

Also i see a lot of duplication in GuessButtonHandler#actionPerformed in body of your if and else branches. Extract it to methods and re-use it.

asked	3 years ago
viewed	5589 times
active	2 years ago

current community

your communities

more stack exchange communities

Number-guessing game in Java with Swing UI

2 Answers 2

Your Answer

Not the answer you're looking for? Browse other questions tagged java game swing number-guessing-game or ask your own question.

Hot Network Questions

current community

your communities

more stack exchange communities

Number-guessing game in Java with Swing UI

2 Answers 2

Did you find this question interesting? Try our newsletter

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged java game swing number-guessing-game or ask your own question.

Related

Hot Network Questions