Guess the random number game

I have created the classic "guess the random number" game:

Generator Interface

package ishan.guessingGame.randomNumberGenerators;

 public interface RandomNumber {

 public int generate();
}

Generator Implementation

package ishan.guessingGame.randomNumberGenerators;

import java.util.Random;

public class SimpleRandomNumberGenerator implements RandomNumber {

private int limit=50;

public SimpleRandomNumberGenerator(){

}
public SimpleRandomNumberGenerator(int limit){
    this.limit=limit;
}
@Override
public int generate() {

    return generateRandomNumber();
}

private int generateRandomNumber(){
    Random random=new Random();
    return random.nextInt(limit+1);
}
}

Game interface

package ishan.guessingGame.game;        

public interface Game {

public void startGame();
}

A Game Impl

```
package ishan.guessingGame.game;

import java.util.Scanner;

import ishan.guessingGame.randomNumberGenerators.RandomNumber;
import ishan.guessingGame.randomNumberGenerators.SimpleRandomNumberGenerator;

public class SimpleGuessingGame implements Game{

private int randomNumber;
private int difficultyLevel;
private Scanner scanner;
private RandomNumber random;
private int input=999;

public SimpleGuessingGame(){
scanner = new Scanner(System.in);
}

@Override
public void startGame() {
init();
play();
endGame();
}

private void init(){

System.out.println("Enter The Difficulty : 1 = Easy , 2 = Moderate , 3 =Difficult");

String difficulty;
difficulty=scanner.next();

if(!checkInputIsNumber(difficulty, true)){
endGame();
}

difficultyLevel=Integer.parseInt(difficulty);

if(difficultyLevel == 1){
random=new SimpleRandomNumberGenerator();
}
else if (difficultyLevel == 2){
random=new SimpleRandomNumberGenerator(100);
}
else{
random=new SimpleRandomNumberGenerator(200);
}
randomNumber=random.generate();
}

private void play(){
System.out.println("Input

Looking at the class SimpleRandomNumberGenerator :

• Having generate() delegate to a private method is, in this case a bit overkill.

• You create a new Random instance for each method invocation. This makes the SimpleRandomNumberGenerator not genrating an even distirbution of random numbers. Make it a private final field initialized at the declaration or in the constructor.

• Good use of coding to interfaces, by hiding the implementation details behind an interface.

Then MainApplication :

• Spot on : kept simple this class' responsibility is to wire everything together and kick things off.

And now for the meat SimpleGuessingGame :

• Several fields of this class can be truned into local variables in the one method that uses them : difficultyLevel, random and input.

• startGame() is well implemented, easy to read.

-
On the whole the class has at least 2 responsibilities :

• handling user interaction

• keeping track of game state

That is too much. Imagine here you'd want to be able to play the same game but with a graphical UI.

So we want to split this into more classes using interfaces to hide implementation from one another.

I would suggest using these two interfaces :

interface UserInteraction {

    String askDifficulty();

    void close();

    String askToGuess();

    void showHowToEndMessage();

    void showWinMessage();

    void showGuessSmallerMessage();

    void showGuessLargerMessage();
}

interface GameState {

    Result guess(int guess);
}

public enum Result {

    CORRECT, GUESS_LARGER, GUESS_SMALLER
}

-
checkInputIsNumber() should be split into two separate methods : one for the difficulty and one for the guesses. They can then be named more aptly to increase readability (boolean parameters say so little at the call site)

• the loop in the play() method is implemented as an infinite loop, while it is obviously driven by the game state. Meanwhile endGame() quite harshly exits, this makes it harder to embed the game in a larger program. Only the MainApplication class should call System.exit()

There's more to do as a follow up on these remarks, but I'd start there.

StackExchange Code Review Q#68769, answer score: 5