patternjavaModerate
"Computer guesses my number" game
Viewed 0 times
guessesnumbercomputergame
Problem
In this exercise I have to do this:
Modify the program so that no matter what number the user thinks of (1-100) the computer can guess it in 7 or less guesses.
I'm just a beginner in Java and I'd like to know what you think about my code:
I think it's too slow, because sometimes it takes the computer more than 7 times to guess a number.
Modify the program so that no matter what number the user thinks of (1-100) the computer can guess it in 7 or less guesses.
I'm just a beginner in Java and I'd like to know what you think about my code:
public static void main(String[] args) {
Scanner in = new Scanner(System.in);
Random rand = new Random();
int randNum = 0;
int upperLimit = 100;
int lowerLimit = 1;
String myAnswer = "";
do {
randNum = rand.nextInt(upperLimit - lowerLimit + 1) + lowerLimit;
System.out.println("I think it's " + randNum);
myAnswer = in.nextLine();
if(myAnswer.equals("tl")) { //too low
lowerLimit = randNum + 1;
}
else if(myAnswer.equals("th")) { // too high
upperLimit = randNum - 1;
}
}while(!myAnswer.equals("y"));
in.close();
System.out.println("YAAAY! :D");
}I think it's too slow, because sometimes it takes the computer more than 7 times to guess a number.
Solution
The guessing takes too long because picking a random number is not the best idea.
For example if the number to guess is 100,
and the computer is really really unlucky,
it might pick the numbers 1, 2, 3, 4, 5, 6, ... and take 100 guesses!
A better way is to halve the intervals.
That is, if you know the number must be between 1 and 100, then pick 50.
If that's too high, and so you know the number is between 1 and 50,
then pick 25. And so on.
So, changing one line could make a huge difference:
This is called a binary search.
As @cbojar pointed out in comments,
since binary search takes \$\log_2(n)\$ tests where \$n\$ is the total number of objects, and since \$\log_2(100) = 6.643856\$,
that means it's guaranteed that this algorithm will find the correct number in at most 7 steps.
The string literals "th", "tl", "y" that control the flow of the program are buried inside the code.
If you read this program a month from now,
it might not be obvious how it works until you read the details.
Something that would help with that,
and a good practice is to extract these string literals to constants,
for example:
Now all these magical strings are easy to see near the top.
Should you decide to use different values,
you could just edit them right at the top, simple and clear.
UPDATE
As an extra touch,
as @Tom pointed out in a comment,
it might be a good / interesting idea to make these
This is of course less efficient than a simple
so don't use it excessively everywhere,
only when justified.
These initializers are redundant:
Because, these variables will always be assigned before they are used.
As such, you can remove the initializers:
It's a cosmetic thing,
but your formatting doesn't follow the standard in a few places.
Instead of this:
It should be like this:
And instead of this:
It should be like this:
Again, these are minor issues, but as your programs get larger and larger these little details can add up and seriously hurt readability.
Use editor environments like Eclipse or IntelliJ or Netbeans that can automatically reformat your code following the standard.
For example if the number to guess is 100,
and the computer is really really unlucky,
it might pick the numbers 1, 2, 3, 4, 5, 6, ... and take 100 guesses!
A better way is to halve the intervals.
That is, if you know the number must be between 1 and 100, then pick 50.
If that's too high, and so you know the number is between 1 and 50,
then pick 25. And so on.
So, changing one line could make a huge difference:
// randNum = rand.nextInt(upperLimit - lowerLimit + 1) + lowerLimit;
randNum = (upperLimit + lowerLimit + 1) / 2;This is called a binary search.
As @cbojar pointed out in comments,
since binary search takes \$\log_2(n)\$ tests where \$n\$ is the total number of objects, and since \$\log_2(100) = 6.643856\$,
that means it's guaranteed that this algorithm will find the correct number in at most 7 steps.
The string literals "th", "tl", "y" that control the flow of the program are buried inside the code.
If you read this program a month from now,
it might not be obvious how it works until you read the details.
Something that would help with that,
and a good practice is to extract these string literals to constants,
for example:
private static final String TOO_HIGH_MARKER = "th";
private static final String TOO_LOW_MARKER = "tl";
private static final String CORRECT_MARKER = "y";
// ...
public static void main(String[] args) {
// ...
do {
// ...
if (myAnswer.equals(TOO_LOW_MARKER)) { //too low
lowerLimit = randNum + 1;
} else if (myAnswer.equals(TOO_HIGH_MARKER)) { // too high
upperLimit = randNum - 1;
}
// ...
} while (!myAnswer.equals(CORRECT_MARKER));Now all these magical strings are easy to see near the top.
Should you decide to use different values,
you could just edit them right at the top, simple and clear.
UPDATE
As an extra touch,
as @Tom pointed out in a comment,
it might be a good / interesting idea to make these
equals comparisons a bit more fault tolerant by making them case insensitive by changing to equalsIgnoreCase, for example:if (myAnswer.equalsIgnoreCase(TOO_LOW_MARKER)) { //too lowThis is of course less efficient than a simple
equals,so don't use it excessively everywhere,
only when justified.
These initializers are redundant:
int randNum = 0;
String myAnswer = "";Because, these variables will always be assigned before they are used.
As such, you can remove the initializers:
int randNum;
String myAnswer;It's a cosmetic thing,
but your formatting doesn't follow the standard in a few places.
Instead of this:
if(myAnswer.equals("tl")) { //too low
lowerLimit = randNum + 1;
}
else if(myAnswer.equals("th")) { // too high
upperLimit = randNum - 1;
}It should be like this:
if (myAnswer.equals("tl")) { //too low
lowerLimit = randNum + 1;
} else if (myAnswer.equals("th")) { // too high
upperLimit = randNum - 1;
}And instead of this:
}while(!myAnswer.equals("y"));It should be like this:
} while (!myAnswer.equals("y"));Again, these are minor issues, but as your programs get larger and larger these little details can add up and seriously hurt readability.
Use editor environments like Eclipse or IntelliJ or Netbeans that can automatically reformat your code following the standard.
Code Snippets
// randNum = rand.nextInt(upperLimit - lowerLimit + 1) + lowerLimit;
randNum = (upperLimit + lowerLimit + 1) / 2;private static final String TOO_HIGH_MARKER = "th";
private static final String TOO_LOW_MARKER = "tl";
private static final String CORRECT_MARKER = "y";
// ...
public static void main(String[] args) {
// ...
do {
// ...
if (myAnswer.equals(TOO_LOW_MARKER)) { //too low
lowerLimit = randNum + 1;
} else if (myAnswer.equals(TOO_HIGH_MARKER)) { // too high
upperLimit = randNum - 1;
}
// ...
} while (!myAnswer.equals(CORRECT_MARKER));if (myAnswer.equalsIgnoreCase(TOO_LOW_MARKER)) { //too lowint randNum = 0;
String myAnswer = "";int randNum;
String myAnswer;Context
StackExchange Code Review Q#69329, answer score: 10
Revisions (0)
No revisions yet.