“Guess my Number” in Java

Question

I have just started learning Java, and don't want to start any bad habits, so please review my latest game code:

import java.util.Random;
import java.util.Scanner;
class GuessMyNumber {
    public static void main(String args[]) {
        Random random = new Random();
        Scanner input = new Scanner(System.in);
        int MIN = 1;
        int MAX = 100;
        int comp = random.nextInt(MAX - MIN + 1) + MIN;
        int user;
        int guesses = 0;
        do {
            System.out.print("Guess a number between 1 and 100: ");
            user = input.nextInt();
            guesses++;
            if (user > comp)
                System.out.println("My number is less than " + user + ".");
            else if (user < comp)
                System.out.println("My number is greater than " + user + ".");
            else
                System.out.println("Well done! " + comp + " was my number! You guessed it in " + guesses + " guesses.");
        } while (user != comp);
    }
}

Answer 1

So although this code is a small example it still can benefit from having a few methods sprinkled in to help clear up what you are trying to do. For instance what if you decided that you wanted to change the max and min to be entered in as a parameter to starting the program? You would have to change max and min to be equal to the int value of args[0] and args[1] although for a small example that is not a big deal, but it would be a little more simple to of had a method similar to this.

//gets deafult secret number
private int getSecretNumber(){
    int MIN = 1;
    int MAX = 100;

    return  getSecretNumber(MAX, MIN);
}
//converts a string variable to a int and returns a secret number
private int getSecretNumber(String strMax, String strMin){
    //code to convert string to int
    getSecretNumber(max, min);
}
//generates a secret number between 2 numbers
private int getSecretNumber(int MAX, int MIN){
    return random.nextInt(MAX - MIN + 1) + MIN;
}

Now you will have saved your self some time in the future if/when the design requirements change. if implemented correctly you might not have any code changes.. say something like this.

public static void main(String args[]) {
    int comp = getSecretNumber(); //gets default
    if (args.length == 2)
        comp = getSecretNumber(args[0], args[1]);

    //etc...

It is always a good idea to seperate your business code and your logic code. The business code is the rules for your code. "Do A,B,C give me D, and Save to E" the logic code is the nitty gritty details of how to do A-E. Everyone calls it something different but the concept is always the same. Keep the two as seperate as possible it makes changing your code easier.

Those are my tips. Other than that I don't see any problems with your code other than you don't have a way to escape the loop other than guessing the correct number.

Answer 2

First of all you should move all the game logic outside of the main method into the class itself, this will allow you to easily input optional values into the constructor:

GuessMyNumber(int min, int max)

The variables max and min should be declared as constant values inside the class:

final int MIN;
final int MAX;

The program would also become more readable if it was split into singular responsibility methods. Some methods that come to mind are:

int getUserInput()
void generateNewNumber()
boolean checkUserGuess(int guess) 

I would also check if the input of the user is an integer with a method isInteger(String s) as input.nextInt() will throw an exception if characters are entered.

public static boolean isInteger(String s) {
    try { 
        Integer.parseInt(s); 
    } catch(NumberFormatException e) { 
        return false; 
    }
    return true;
}

My code for this program would have been:

import java.util.Random;
import java.util.Scanner;

class GuessMyNumber {

    // Min and Max values of number generated
    private final int MIN;
    private final int MAX;

    //The number in which to guess
    private int guessnumber = 0;
    //Total guesses taken
    private int guessestaken = 0;

    public GuessMyNumber(int min, int max) {
        MIN = min;
        MAX = max;

        //Assign a new random number to guessnumber
        guessnumber = generateNewNumber();

        int guess;

        do {
            guessestaken++;

            // Get the user input whilst guess is wrong
            guess = getUserInput();

            //Check user input
        } while (!checkUserGuess(guess));
    }

    // Gets an integer value from user
    private int getUserInput() {
        Scanner input = new Scanner(System.in);
        String userinput;
        do {
            System.out.print("Guess a number between 1 and 100: ");
            // Make the user input a value while input is not an integer
            userinput = input.nextLine();
        } while (!isInteger(userinput));
        //Returns the input parsed as an integer
        return Integer.parseInt(userinput);
    }

    //Generate a new value between MIN and MAX
    private int generateNewNumber() {
        Random random = new Random();
        return random.nextInt(MAX - MIN + 1) + MIN;
    }

    private boolean checkUserGuess(int guess) {
        if (guess == guessnumber) {
            //If the user guesses right return true
            System.out.println("Well done! " + guessnumber + " was my number! You guessed it in " + guessestaken + " guesses.");
            return true;
        } else {
            // Return false with appropriate output to console
            if (guess > guessnumber) {
                System.out.println("My number is less than " + guess + ".");
            } else {
                System.out.println("My number is greater than " + guess + ".");
            }
        }
        return false;

    }

    //Returns true when input string is a valid integer
    public static boolean isInteger(String s) {
        try {
            Integer.parseInt(s);
        } catch (NumberFormatException e) {
            return false;
        }
        return true;
    }

    public static void main(String args[]) {
        new GuessMyNumber(0, 100);
    }
}

Answer 3

You declare variables MAX and MIN using a notation for constants. If they are meant to be constants, they should be declared as constant fields on the class:

static final int MAX = 100;
static final int MIN = 1;

Otherwise:

int max = 100;
int min = 1;

Variable declarations: you should declare variables nearest to where you will be using them, and inline if used in only one place.

Instead of declaring: Random random = new Random(), just use the static method from Random: Random.nextInt(MAX) + 1

Increment guesses where its being used, in the message String construction.

When generating the random number, just pass in MAX. The range will be [0-MAX), then add MIN if needed.