1
\$\begingroup\$

This program is one of the exercises I solved in my book. Can anyone suggest any improvements to it?

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

public class GuessMyNumber {

  public static void main(String[] args) {
    Scanner in = new Scanner(System.in);
    Random random = new Random();

    System.out.println("I'm thinking of a number between 1 and 100 (including both).\nCan you guess what it is?");
    System.out.print("Type a number : ");
    int guessNum = in.nextInt();
    System.out.printf("Your guess is : %d\n", guessNum);
    int randomNum = random.nextInt(100) + 1;
    System.out.printf("The number I was thinking of is : %d\n", randomNum);
    System.out.print("You were off by : ");

    int diff = guessNum - randomNum;
    if (guessNum >= randomNum) {
      System.out.println(diff);
    } else { System.out.println(randomNum - guessNum); }
  }
}
\$\endgroup\$

migrated from stackoverflow.com Sep 10 '16 at 13:48

This question came from our site for professional and enthusiast programmers.

4
\$\begingroup\$
    int diff = guessNum - randomNum;
    if (guessNum >= randomNum) {
      System.out.println(diff);
    } else { System.out.println(randomNum - guessNum); }

You mix styles here. Consider

    if (guessNum >= randomNum) { System.out.println(guessNum - randomNum); }
    else { System.out.println(randomNum - guessNum); }

That's a distinct style. It puts the {} on the same line as the contained statement and it does every calculation just when it is needed. Most importantly it's consistent.

I don't like that style. It's especially uncommon. I'd prefer

    int difference = guess - solution;
    if (difference >= 0) {
        System.out.println(difference + " high.");
    } else {
        System.out.println(-difference + " low.");
    }

We only calculate the difference once. Then we use it three times.

This uses a four-column indent, which is standard in Java.

This puts {} on the same lines as their control structures when possible. And it puts the enclosed statements on lines of their own. Also standard in Java.

I changed the names of the variables to concentrate on their role in the program. Not characteristics of how they are defined, e.g. randomly or numeric. This leaves us with more flexibility to change in the future. For example, this version could be easily modified to guess letters.

If you don't like the high/low, you could just say

    System.out.println(Math.abs(guessNum - randomNum));

No intermediate variables or if/else that way.

\$\endgroup\$
1
\$\begingroup\$
  1. You can calculate the difference between 2 numbers by using the abs() method in the built in Java Math library.

    So instead of:

    int diff = guessNum - randomNum;
    if (guessNum >= randomNum) {
        System.out.println(diff);
    } else { System.out.println(randomNum - guessNum); }
    

    You would simply do:

    System.out.println(Math.abs(randomNum - guessNum));
    
  2. I would have an if-else statement that checks if the user guessed correctly and print out a message such as "Congrats! Your guess was correct!" instead of always printing out the difference in the guess and correct answer which would only let them know they got the right answer if it was 0 (this isn't very exciting)

  3. The next suggestion is moving some of this logic out to query methods. This is kinda silly for such a small program but the concept is important for more complex operations as well as program readability.

    The 2 query methods that pop out at me are check if the person guessed the correct number and if they didn't what is the difference between the correct answer and what they guessed.

    public boolean isCorrectGuess(int guess, int secretNum) {
        return guess == secretNum;
    }
    
    public int getAbsDiff(int num1, int num2) {
        return Math.abs(num1 - num2);
    }
    

    The complete program would look something like this:

    import java.util.Random;
    import java.util.Scanner;
    
    public class GuessMyNumber {
    
        public static void main(String[] args) {
            Scanner in = new Scanner(System.in);
            Random random = new Random();
    
            System.out.println("I'm thinking of a number between 1 and 100 (including both).\nCan you guess what it is?");
            System.out.print("Type a number : ");
            int guessNum = in.nextInt();
            System.out.printf("Your guess is : %d\n", guessNum);
            int secretNumber = random.nextInt(100) + 1;
            System.out.printf("The number I was thinking of is : %d\n", randomNum);
    
            if (isCorrectGuess(guessNum, secretNum) {
                System.out.print("You Win!");
            } else {
                System.out.print("You were off by " + getDiff(guessNum, secretNum));
            }
        }
    
        public boolean isCorrectGuess(int guess, int secretNum) {
            return guess == secretNum;
        }
    
        public int getDiff(int num1, int num2) {
            return Math.abs(num1 - num2);
        }
    
    }
    
\$\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.