codeinprogress
codeinprogress

Reputation: 3501

Random number generator generates duplicates

Framework: Java

public static List<Integer> buttonIdList = new ArrayList();

public void myMainMethod() {
   for(Integer i = 0; i < 11; i++) {
      int randomButtonId = getUniqueIdNumber();
   }
}

private static Integer getUniqueIdNumber() {
        Random ran = new Random();
        int randomButtonId = ran.nextInt(20) + 1;

        if(buttonIdList.contains(randomButtonId)) {
            getUniqueIdNumber();
        } else {
            buttonIdList.add(randomButtonId);
        }

        return randomButtonId;
    }

When the code encounters a duplicate, it calls itself (recursively) and in the second try if the number is unique the return statement returns it to the myMainMethod or to getUniqueIdNUmber?

Where should the return statement be placed?

Upvotes: 0

Views: 72

Answers (1)

Eran
Eran

Reputation: 394126

You should return the result of the recursive call:

private static Integer getUniqueIdNumber() {
    Random ran = new Random();
    int randomButtonId = ran.nextInt(20) + 1;

    if(buttonIdList.contains(randomButtonId)) {
        return getUniqueIdNumber();
    } else {
        buttonIdList.add(randomButtonId);
    }

    return randomButtonId;
}

P.S., it would be better to make Random ran a static variable instead of creating a new Random instance in each recursive call.

private static Random ran = new Random();
private static Integer getUniqueIdNumber() {
    int randomButtonId = ran.nextInt(20) + 1;
    if(buttonIdList.contains(randomButtonId)) {
        return getUniqueIdNumber();
    } else {
        buttonIdList.add(randomButtonId);
        return randomButtonId;
    }
}

And you might consider changing buttonIdList to a HashSet (or LinkedHashSet if you care about insertion order) in order to make the search for existing number more efficient.

Upvotes: 7

Related Questions