Guess My Number Game in Java

Question

Basically, what this program does is that this generates a random number, and asks the user to guess it with multiple chances. So, if the user is close, it says that the user is close, and if the user is not close, then it says that the user is too high/low and if the user guesses it right, it congratulates the user.

So, you must now be wondering what I expect from this code review. Well I expect these stuff:

• I expect you to point out flaws
• I expect you to identify spots which are inefficient and can be optimised.
• I expect you to comment on the use of concepts and how I have not used them right.

Main.java:

package com.GuessMyNumber;

import java.util.Scanner;

public class Main {

    public static void main(String[] args) {
        GuessMyNumber guessMyNumber = new GuessMyNumber();
        Scanner scanner = new Scanner(System.in);
        int input;

        guessMyNumber.generateNumber(1, 75);

        System.out.println("Hey there! Let's play Guess my number! Start guessing my number once you're ready.");

        while (!guessMyNumber.numberFound) {
            input = Integer.parseInt(scanner.next());
            System.out.println(guessMyNumber.validateInput(input));
        }
    }
}

GuessMyNumber.java:

package com.GuessMyNumber;

import java.util.Random;

public class GuessMyNumber {
    int randomNumber;
    String response;
    boolean numberFound = false;

    public GuessMyNumber()
    {

    }

    public int generateNumber(int min, int max)
    {
        Random random = new Random();

        this.randomNumber = random.nextInt((max - min) + 1) + min;

        return this.randomNumber;
    }

    public String validateInput(int guessedNumber)
    {
        if(guessedNumber < this.randomNumber && (this.randomNumber - guessedNumber) < 5) {
            this.response = "You're close!";
        }
        if(guessedNumber < this.randomNumber && (this.randomNumber - guessedNumber) > 5) {
            this.response = "You're too low!";
        }
        if(guessedNumber > this.randomNumber && (guessedNumber - this.randomNumber) < 5) {
            this.response = "You're close!";
        }
        if(guessedNumber > this.randomNumber && (guessedNumber - this.randomNumber) > 5) {
            this.response = "You're too high!";
        }
        if(guessedNumber == this.randomNumber) {
            this.response = "Congratulations! You guessed my number!";
            this.numberFound = true;
        }

        return this.response;
    }
}

Answer 1

Robustness

The randomNumber field of GuessMyNumber should not be public. For example a bad user could do this:

GuessMyNumber guessMyNumber = new GuessMyNumber();
guessMyNumber.generateNumber(1, 75);
guessMyNumber.randomNumber = 25;
guessMyNumber.validateInput(25);  // -> yeay, I win!

That would be devastating. Internal implementation details like this should be hidden, make them private. The same goes for numberFound.

It would be better to remove the generateNumber method, and initialize the number once and for all in the constructor. That is, randomNumber can be final, which will make your class more robust, less prone to errors. If you want to play multiple games, you can create a new instance of GuessMyNumber.

Finally, the response variable really doesn't need to be an instance variable, it should be a local variable in the validateInput method. In fact, you don't even need to store the message in a local variable, you can simply return directly.

Simplify

The conditions in validateInput have many duplicated elements. It would be better to cache the difference of the guess and the target number in a variable and reuse it.

Also some of the conditions are mutually exclusive, so you should chain them using else if statements.

Unit testing

To make sure that the validation logic works well and correctly handles all corner cases (too high, too low, close, perfect match), it would be good to unit test it.

Coding style

The convention in Java is to put opening braces on the same line as the statement, so instead of writing like this:

public String validateInput(int guessedNumber)
{

I recommend to adopt this writing style:

public String validateInput(int guessedNumber) {

Suggested implementation

Putting the above suggestions together (and then some more):

import org.junit.Test;
import java.util.Random;
import static org.junit.Assert.assertEquals;

class GuessMyNumber {

    public static final int CLOSE_LIMIT = 5;
    public static final String MSG_TOO_HIGH = "You're too high!";
    public static final String MSG_TOO_LOW = "You're too low!";
    public static final String MSG_CLOSE = "You're close!";
    public static final String MSG_WIN = "Congratulations! You guessed my number!";

    private final int randomNumber;
    private boolean numberFound = false;

    public GuessMyNumber(int min, int max) {
        Random random = new Random();
        this.randomNumber = random.nextInt((max - min) + 1) + min;
    }

    public String validateInput(int guessedNumber) {
        int diff = guessedNumber - randomNumber;
        if (diff == 0) {
            this.numberFound = true;
            return MSG_WIN;
        }
        int absoluteDiff = Math.abs(diff);
        if (absoluteDiff <= 5) {
            return MSG_CLOSE;
        }
        if (diff < 0) {
            return MSG_TOO_LOW;
        }
        return MSG_TOO_HIGH;
    }
}

public class GuessMyNumberTest {
    private static final int NUMBER = 55;

    private final GuessMyNumber game = new GuessMyNumber(NUMBER, NUMBER);

    @Test
    public void testTooHigh() {
        assertEquals(GuessMyNumber.MSG_TOO_HIGH, game.validateInput(NUMBER + GuessMyNumber.CLOSE_LIMIT + 1));
    }

    @Test
    public void testTooLow() {
        assertEquals(GuessMyNumber.MSG_TOO_LOW, game.validateInput(NUMBER - GuessMyNumber.CLOSE_LIMIT - 1));
    }

    @Test
    public void testCloseHigh() {
        assertEquals(GuessMyNumber.MSG_CLOSE, game.validateInput(NUMBER + GuessMyNumber.CLOSE_LIMIT));
    }

    @Test
    public void testCloseLow() {
        assertEquals(GuessMyNumber.MSG_CLOSE, game.validateInput(NUMBER - GuessMyNumber.CLOSE_LIMIT));
    }

    @Test
    public void testWin() {
        assertEquals(GuessMyNumber.MSG_WIN, game.validateInput(NUMBER));
    }
}

Answer 2

There's a bug that occurs when the guessed number is equal to the random number plus or minus 5. For example, if the random number is 5 and the first guess is 10, we get

Hey there! Let's play Guess my number! Start guessing my number once you're ready.

10

null