# “Guess my number” game in Java Swing

Also posted at rgcsm.info/blog

//Number Guessing Game in Java Swing
//Also posted at

import javax.swing.*;
import java.awt.*;
import java.awt.event.*;

class Guess extends JFrame
{
JTextField t1, t2, t3, t4;
JLabel j4;
ButtonListener bl1;
ButtonListener2 bl2;
ButtonListener3 bl3;

//setting random number in rand variable
int rand=(int) (Math.random()*100);
int count=0;

public Guess()
{
//Get the container
Container c = getContentPane();

//Set absolute layout
c.setLayout(null);

//Set Background Color
c.setBackground(Color.WHITE);

//Creating image
JLabel lblpic = new JLabel("");
lblpic.setIcon(new ImageIcon("images.png"));
lblpic.setBounds(0,0,500,350);

//Creating label Guess my number text
JLabel j=new JLabel("Guess my number game");
j.setForeground(Color.RED);
j.setFont(new Font("tunga",Font.BOLD,24));
j.setSize(270,20);
j.setLocation(300,35);

//Creating label Enter a number.....
JLabel j1=new JLabel("Enter a number b/w 1-100");
j1.setFont(new Font("tunga",Font.PLAIN,17));
j1.setSize(270,20);
j1.setLocation(300,60);

//Creating TextField for input guess
t1=new JTextField(10);
t1.setSize(50,30);
t1.setLocation(350,80);

//Creating Label for Display message
j4=new JLabel("Try and guess my number");
j4.setFont(new Font("tunga",Font.PLAIN,17));
j4.setSize(270,20);
j4.setLocation(290,130);

//Creating Text field for best score
t2=new JTextField(10);
t2.setSize(40,20);
t2.setLocation(10,10);

//Creating Label for best score
JLabel j5=new JLabel("Best Score");
j5.setFont(new Font("tunga",Font.PLAIN,17));
j5.setSize(270,20);
j5.setLocation(60,10);

//Creating guess text field
t3=new JTextField(10);
t3.setSize(40,20);
t3.setLocation(160,10);

//Creating guess Label
JLabel j6=new JLabel("Guesses");
j6.setFont(new Font("tunga",Font.PLAIN,17));
j6.setSize(270,20);
j6.setLocation(210,10);

//creating 3 buttons
JButton b1=new JButton("Guess");
b1.setSize(150,40);
b1.setLocation(260,250);
bl1=new ButtonListener();

JButton b2=new JButton("Give up!");
b2.setSize(100,30);
b2.setLocation(180,200);
bl2=new ButtonListener2();

JButton b3=new JButton("Play Again");
b3.setSize(120,30);
b3.setLocation(330,200);
bl3=new ButtonListener3();

//Place the components in the pane

//Changing TextFields to UnEditable
t2.setEditable(false);
t3.setEditable(false);

//Set the title of the window
setTitle("GUESS MY NUMBER");

//Set the size of the window and display it
setSize(550,350);
setVisible(true);
setDefaultCloseOperation(EXIT_ON_CLOSE);
}

private class ButtonListener implements ActionListener
{
int bestScore=100;
public void actionPerformed(ActionEvent e)
{
int a = Integer.parseInt(t1.getText());
count=count+1;
if(a<rand)
{
j4.setText(a+" is too low, try again!!");
}
else if(a>rand)
{
j4.setText(a+" is too high, try again!!");
}
else
{
j4.setText("CORRECT, YOU WIN!!!!");
if(count<bestScore)
{
bestScore=count;
t2.setText(bestScore+"");
}
else
t2.setText(""+bestScore);
t1.setEditable(false);
}

//setting focus to input guess text field
t1.requestFocus();
t1.selectAll();

t3.setText(count+"");
}
}

private class ButtonListener2 implements ActionListener
{
public void actionPerformed(ActionEvent e)
{
t3.setText("");
t1.setText("");
t1.setEditable(false);
}
}

private class ButtonListener3 implements ActionListener
{
public void actionPerformed(ActionEvent e)
{
rand=(int) (Math.random()*100);
t1.setText("");
t3.setText("");
j4.setText("Try and guess my number");
t3.setText("");
count=0;
t1.setEditable(true);
t1.requestFocus();
}
}

public static void main(String[] args)
{
new Guess();
}
}

• Welcome to Code Review! Because links tend to go away over time, please also copy the description from your blog to this post itself. – Simon Forsberg May 3 '14 at 12:31
• Seems like someone needs to read about the EDT. Your code create and mutates Swing components off of the EDT, this is in complete violation of Swings threading policy. – Boris the Spider May 3 '14 at 18:07

# Layoutmanagers

You're using absolute positioning for all your elements which won't scale well when you enlarge or shrink the application. For this reason you should use a layoutmanager that will handle the resizing for you (and it's a lot easier to extend your application with new elements).

//Set the title of the window
setTitle("GUESS MY NUMBER");


The comment doesn't tell me anything I don't know yet. Comments should explain why I do something, not what I'm doing.

# Naming

Your class is named Guess, which isn't very descriptive of what it means. GuessingGame would be a lot better and clarifies that it is a game we're talking about.

# Spacing

Add a space before and after your binary operators like *, &&, =, etc. Right now the code gives a very compressed feeling.

# Braces

I see this line of code:

else
t2.setText(""+bestScore);
t1.setEditable(false);


Always use braces so I can distinguish easily between what should be inside that if statement. Right now I can't tell for sure if that is intended or if it is a logical error. It will also prevent you from introducing errors when you add an expression to your else body without thinking about the braces.

If anything, it's also a matter of staying consistent.

# Main

I prefer to put my main method in a separate class so I always have instant knowledge of where my application starts without having to dig into my separate classes. It's also more comfortable to work with when you have to setup certain things like the "Look and Feel" or logging.

• Man, I had to scroll all the way down to find an answer mentioning the absolute positioning. As someone who has to maintain several legacy GUIs with enormous complexity, this is one of the most terrible things I have to deal with and I find myself rewriting one class after another to get rid of it. – Ingo Bürk May 3 '14 at 21:06

Vogel612's answer already covered naming. I'd like to highlight some other things: some because of how Swing works, and some about initialisation.

# Initialisation

Initialisation and reset take mostly the same actions but are in two different places: ButtonListener3.actionPerformed and Guess.<init>. Try taking these actions out and creating methods that clarify their intent:

private static final int MAX_NUMBER = 100;
public void start() {
rand = (int) (Math.random() * MAX_NUMBER) + 1;
count = 0;
// set labels here
}

public void restart() {
// any cleanup goes here
start();
}


# Keep it on the EDT

Swing is a multi-threaded environment, and any manipulation of GUI elements should happen on the Event Dispatch Thread (EDT). For most of an application's lifetime, this is not a direct concern because it primarily runs inside this EDT, but bootstrapping is a special case and requires your intervention.

public static void main(String[] args)
{
SwingUtilities.invokeLater(
new Runnable() {
public void run() {
Guess guess = new Guess();
guess.start();
}
}
};
}


Or, since Java 8:

public static void main(String[] args)
{
SwingUtilities.invokeLater(
() -> {
Guess guess = new Guess();
guess.start();
}
);
}


Yes, even running a single method or constructor requires this, and you may get away with it most of the time but, as I learned to my shame, not all of the time. ;)

# Layout and Resizing

Not having a layout manager makes it easier to have components right where you want them, but it makes your container ignore resizing. If you've already picked the (only) size you're going to support, you may want to constrain it:

setMinimumSize(getSize());


# Randoms in Java

Math.random() returns a value $\in [0,1[$, so if you want an integer between min and max, inclusive, you'll need to change a bit:

rand = (int) (Math.random() * (max - min + 1)) + min;


...which, for 1..100 would make:

rand = (int) (Math.random() * 100) + 1;


Because I got 0 on my first run of your game. :p

# Seperation of Concerns

The guessing part can be extracted into a class of its own, which will also take care of your magic numbers:

class GuessingGame {
int value;
int tries;
int topScore = Integer.MAX_VALUE;

Random random = new Random();

public void reset() {
tries = 0;
value = random.nextInt(getMaximum() - getMinimum() + 1) + getMinimum();
}

public Result guess(int guess) {
tries++;

if ( guess == value ) {
if ( topScore > tries ) {
topScore = tries;
}
return Result.EXACT;
}

return guess < value ? Result.TOO_HIGH : Result.TOO_LOW;
}

public int getNumberOfTries() {
return tries;
}

public int getTopScore() {
}

public int getMinimum() { return 1; }
public int getMaximum() { return 100; }
}

enum Result {
TOO_HIGH, TOO_LOW, EXACT;
}


### Naming:

JTextField t1, t2, t3, t4;
JLabel j4;
ButtonListener bl1;
ButtonListener2 bl2;
ButtonListener3 bl3;


Better would definitely be:

JTextField guessInput, bestScore, guessText;


You should already be able to see: t4 was unused, guessInput (taken from the comments at initialization) and guessText (also take from the comment at init) are somewhat doubled.

ButtonListener, ButtonListener2, ButtonListener3


Wait what??? you have 3 different action listeners and call them ButtonListener 1, 2 and 3?
Shame on you. Better names would be:

GuessSubmittedListener
SurrenderListener
NewGameListener


Move this thorough your whole code.

### Starting the game:

public static void main(String[] args)
{
new Guess();
}


Conventionally you'd have something like the following instead:

public static void main(String[] args){
Guess game = new Guess();
game.start(); // game.run(); game.begin(); game.whatever();
}


The constructor of game is supposed to initialize it. But it never should be responsible for starting the game.

### Braces:

The convention for Java is "egyptian braces". The opening curly brace is on the line with the block expression, the closing brace is separate. You are using the C# conventions...

### Magic Numbers:

Your whole code is littered with magic numbers, numbers that have no explanation whatsoever. Move these numbers to constants:

int rand = (int) (Math.random()*100);


Nonononon...

private static final int RANDOM_BORDER = 101;

Random rnd = new Random();
int randNumber = rnd.nextInt(MAX_NUMBER);


Same goes for the whole initialization stuff... There's so many magic numbers there I'm almost having a breakdown right now.

• (int) (Math.random()*100); does not do the same as rnd.nextInt(101);. The first does not include the possibility for the number 100, the second does. Other than that, +1. – Simon Forsberg May 3 '14 at 12:28
• You don't mention that lack of Swing threading. This is the biggest issue. The game runs itself on the main thread. This is a massive bug waiting to happen. Otherwise +1 – Boris the Spider May 3 '14 at 18:06

"Give up!" and "Play again!" should be the same button. If you've just won the game, it should be "Play again!", and if you tried and failed and want to reset the game, it should be "Give Up!". If you don't want to change the label every time, "Reset" will work for both scenarios.