Bug in recursive function

Question

I'm trying to get a pin number with only 4 digits. But the numbers that are less than 1000 are also getting printed. What is happening in this code?

import java.util.Random;

public class Util {

    public static int getRandomPINNumber(){
        Random randomGenerator = new Random();
        int randomNo = randomGenerator.nextInt(10000);
        if(randomNo < 1000)
        getRandomPINNumber(); //repeat if it is less than 1000,some bug here
        return randomNo;
    }

    public static void main(String[] args){
        for(int i = 0;i<100; i++)
            System.out.println(getRandomPINNumber());
    }

}

Output

6413
1692
5734
105  <--- why 105 is getting printed, it should reevaluate the pin right? 
4857
6348
1355

Answer 1

The problem is that you are not returning the result of the recursive call. Change you code to:

public static int getRandomPINNumber(){
    Random randomGenerator = new Random();
    int randomNo = randomGenerator.nextInt(10000);
    if(randomNo < 1000)
        return getRandomPINNumber(); //repeat if it is less than 1000
    return randomNo;
}

When you call the function for the first time and a number less than 1000 is generated, you recursively call getRandomPINNumber but ignore the return value.

Also, you should not call new Random() multiple times. Call it once and store the result.

Answer 2

Three things, in order of increasing pedantry

1. You need to return the value of the recursive function: return getRandomPINNumber(); else you're discarding the result.

2. Don't call Random randomGenerator = new Random(); multiple times, else you ruin the statistical properties of the generator. Have randomGenerator as a field of your class, or pass it in to your function.

3. Never discard results of a random number generator else you will introduce some statistical bias. (Over the years very many research papers have been debunked due to improper random number generator usage). In your case you can use int randomNo = randomGenerator.nextInt(9000) + 1000; and drop the discarding altogether.