Guess number game with mines

I've made mine first Java program and it was Guess a number game and got some great code review, so I've decided to first implement that suggestions and to make a game a bit more complex/advanced.

So now, the user can select between 3 options, 3 different intervals (1-10, 1-20, 1-100), but also, in each interval there are few mines he must avoid.

Eclipse suggested to set private int max; to bet private static int max; and some other private int's as well, wasn't sure why?

Let me know if something is not clear in explanation of game.

Here is the code:


Main.java

public class Main {

    public static void main(String[] args) {
        Game game = new Game();
        game.options();
        game.playGame();
    }
}

Game.java

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

public class Game {
private static int min=1;
private static int max;
private static int numberOfMines;
private int randomNumber;
int mine[] = new int[20];
private int numGuesses;
private int userGuess;

public static int promptForInteger() {
Scanner input = new Scanner(System.in);
while(!input.hasNextInt()) {
System.out.println("Please enter valid number");
input.next();
}
return input.nextInt();
}

public void options(){
int option = 0;
do{
System.out.println("Please select your game: ");
System.out.println("1) Guess number between 1 and 10 with 1 mine.");
System.out.println("2) Guess number between 1 and 20 with 3 mines.");
System.out.println("3) Guess number between 1 and 100 with 20 mines.");
option = promptForInteger();

}while (option == 0 || option > 3);

switch (option) { // set max depending on user selection
case 1: max = 10;
numberOfMines = 0; // one mine
break;
case 2: max = 20;
numberOfMines = 2; // 3 mines

Potential bugs

When asking the user to select their option for the game, negative values aren't checked:

do{
System.out.println("Please select your game: ");
System.out.println("1) Guess number between 1 and 10 with 1 mine.");
System.out.println("2) Guess number between 1 and 20 with 3 mines.");
System.out.println("3) Guess number between 1 and 100 with 20 mines.");
option = promptForInteger();

}while (option == 0 || option > 3);

If the user were to enter -7 for example, it would not match option == 0 || option > 3 so the while loop will exit. This will make the rest of the code crash because numberOfMines and max were not initialized.

Also, in the askForGuess() method, whose purpose is to ask the user for a guess, the code isn't checking that what the user entered is a valid integer. You already have a promptForInteger() method exactly for that! You can re-use it:

public void askForGuess(){
     System.out.println("You are guessing random number between 1 and " + this.max + ". Enter your guess: ");
     userGuess = promptForInteger();
 }

Reuse Random objects

To generate a random number, your current method is:

public static int getRandom() {
    Random rand = new Random();
    return rand.nextInt((max - min) + 1) + min; 
}

The issue with this is that, everytime the method is called, a new Random object is created. This produces mediocre quality random numbers. A Random object should be created just once, and be re-used. For example, you could make it a constant of the Game class.

Another possibility is to use the ThreadLocalRandom class, which makes it really easy to generate a random integer in a range:

public static int getRandom() {
    return ThreadLocalRandom.current().nextInt(min, max + 1);
}

(We need to have max + 1 since the upper bound is exclusive).

Use static only for global fields

A lot of the code is static:

private static int min=1;
private static int max;
private static int numberOfMines;

along with the methods getRandom() and promptForInteger(). Static fields are generally not a good idea. min, max and numberOfMines being static means that if we create two games, they will have the same value. This is probably not what you want: each game should have its own number of mines, etc.

As such, consider making them instance fields. You will also need to change getRandom() and make it an instance method since it operates on those instance fields.

promptForInteger() can also be made an instance method.

Unused code

private int numGuesses;

is unused in your code. Either remove it or use it, unused code shouldn't be left in code.

Hard-coded bounds, off-by-one checks

The maximum number of mines is hard-coded at multiple places of the code:

int mine[] = new int[20];
// ...
for (int j = i+1; j<19; j++)

This shows a small design issue: you are always creating a mine array of length 20 because that is the maximum the game can have. But when the game has less mines than 20, then the array is storing elements that will be of no use. Instead, this array should be instantiated with the correct number of elements, which is the number of mines in the game.

Additionally, numberOfMines, despite its name, does not represent the number of mines:

switch (option) { // set max depending on user selection
case 1: max = 10;
        numberOfMines = 0; // one mine
        break;
case 2: max = 20;
        numberOfMines = 2; // 3 mines
        break;
case 3: max = 100;
        numberOfMines = 19; // 20 mines
        break;
}

This is awkward, why would numberOfMines be equal to 2 when there are 3 mines? Consider changing that so that numberOfMines accurately represents the number of mines in the game, instead of the number minus 1.

Your game has 3 options, each of them with a specific minimum value, maximum value and number of mines. Those values are hard-coded in the options printed to the user and in the switch.

You could create an enum of those option, which would look like

enum Options {
    EASY(1, 10, 1),
    MEDIUM(1, 20, 3),
    HARD(1, 100, 20);

    private final int min, max, numberOfMines;

    Options(int min, int max, int numberOfMines) {
        this.min = min;
        this.max = max;
        this.numberOfMines = numberOfMines;
    }
}

This would centralize all the options in one place. You can then loop over the options to print them and select the option chosen by the user (for example taking its ordinal as a first step).

Using the Stream API

The mine array is populated with random integers from min to max, with the exception from the first guess and the number to guess. Instead of having a while loop, you can do it easily using the Stream API:

```
mine = ThreadLocalRandom.current()
.ints(min - 1, max)
.filter(i -> i != userGuess && i != randomNumber)
.distinct()

StackExchange Code Review Q#132152, answer score: 7