Guess Number Game - Console based game using Java

Question

This game is pretty common for beginner projects. I wrote a version of this before all in Main - so here I challenge myself to recreate it in a more OO style.

I wanted to take a more OO approach, so any feedback on where I could do better or am making some errors is appreciated. I also have some personal confusion if in some parts I'm coding this procedurally. I might be mistaken but I think this code is a bit of both styles, and if so, can someone help explain the distinctive differences? Is there too much recursion in some parts? How does the readability appear? What are your thoughts?

The game will generate a random secret number and ask the user to input their guesses within the given number of attempts Code below:

import java.util.InputMismatchException; import java.util.Random; import java.util.Scanner; public class GuessGame { private static final Scanner SC = new Scanner(System.in); private static final Random R = new Random(); private static int secret = R.nextInt(100) + 1; private static final int GIVEN_ATTEMPTS = 10; private int attempts = 0; public void playGame() { promptGuess(); } private int promptGuess() { try { printLine("Guess a number from 1-100"); int guess = SC.nextInt(); if (guess < 1) { promptGuess(); } else { return compare(guess); } } catch(InputMismatchException e) { printLine("Only numbers please"); SC.nextLine(); promptGuess(); } return 0; } private int compare(int guess) { attempts++; if (attempts == GIVEN_ATTEMPTS) { return promptResult(4); } int difference = secret - guess; if (difference > 0) { return promptResult(1); } else if (difference < 0) { return promptResult(2); } else { return promptResult(3); } } private int promptResult(int n) { switch (n) { case 1: printLine("Guess was too low, try again"); promptGuess(); break; case 2: printLine("Guess was too high, try again"); promptGuess(); break; case 3: printLine("You got it! Nice guess! The number was " + secret); printLine("In " + attempts + " attempts"); printLine("Game Over"); attempts = 0; secret = R.nextInt(100) + 1; playAgain(); case 4: printLine("Too many guesses! " + GIVEN_ATTEMPTS + "/" + GIVEN_ATTEMPTS + " used."); printLine("Secret number was: " + secret + " Game Over"); attempts = 0; secret = R.nextInt(100) + 1; playAgain(); } return 0; } private void printLine(String s) { System.out.println(s); } private void playAgain() { printLine("Play again? (Enter Y for yes or N for no)"); String reply = SC.next(); if (reply.equalsIgnoreCase("Y")) { playGame(); } else if (reply.equalsIgnoreCase("N")) { printLine("Thanks for playing"); System.exit(0); } else { System.out.println("Please enter a valid answer"); playAgain(); } } } public class Main { public static void main(String[] args) { GuessGame g = new GuessGame(); g.playGame(); } } 

Answer 1

Curious, this code looks a lot like the code in this question. I'm not saying that you have another account, but you don't happen to be in the same class or something?

---

Your formatting seems to be inconsistent, use an automatic code formatter (your IDE most likely has one).

---

private static final Scanner SC = new Scanner(System.in); 

Don't shorten names just because you can, it makes the code harder to understand and maintain.

The usage of the scanner (or streams in general) should most likely be scoped. Streams are rather easily associated with native resources which must be destroyed explicitly to be freed.

---

 private static final Scanner SC = new Scanner(System.in); private static final Random R = new Random(); private static int secret = R.nextInt(100) + 1; private static final int GIVEN_ATTEMPTS = 10; private int attempts = 0; 

Some of your state is static, your methods are all instance. You most likely want to make all your state instance.

---

 if (guess < 1) { promptGuess(); } else { return compare(guess); } 

You're recursing here, so keeping to hit Return without entering something should at some point crash your game because of a stackoverflow.

---

if (attempts == GIVEN_ATTEMPTS) { 

A check for greater-quals would be safer.

---

private int promptResult(int n) { 

This does not prompt for a result, it displays whether the guess was correct or not, the method should be renamed.

---

 switch (n) { case 1: 

Instead of using numbers here, you could either use constants or an enum. That way nobody would need to remember that 2 means "too high".

---

 private void printLine(String s) { System.out.println(s); } 

Good idea, but you're gaining very, very little when it comes to readability with this method.

---

System.exit(0); 

Something to keep in mind is that System.exit is not "exit the application" but "kill the JVM process". When invoking this not even finally blocks may run. In this case it does not matter, but something to keep in mind.

---

Your logic is all over the place. Instead of chaining the methods together like you did, you should consider having a main method which handles the flow and calls the methods as needed. That way you'd have the logic of your game in one place but the "details" are hidden away in functions, which would make it rather easy to not only figure out how the game works, but also makes it easier possible to change single functions.

Regarding on how to structure this more proper, there is not much to do except cleaning up the mixed state. What you can start with is to have the logic of the game separated from the logic to print to the terminal. So you'd have a Game class and that one would accept a GameUi interface as parameter, like this:

public interface GameUi { public abstract int getGuess(); // ... } public class SimpleTerminalGameUi implements GameUi; public class Game { public Game(GameUi gameUi); // Somewhere in your logic: int guess = gameUi.getGuess(); } new Game(new SimpleTerminalGameUi()); 

That would allow you to decouple the presentation from the internal state of the game.

As further exercise, do the same with the secret provider.

I might be mistaken but I think this code is a bit of both styles, and if so, can someone help explain the distinctive differences?

Your code is not object-oriented at all, you're just using a single object as container. But to be fair, there isn't that much logic to shell out to different objects to begin with.