CODEWITHSUNDEEP
javaandroidmathrandom
Android is everything for me
Android is everything for me

Reputation: 3791

Bug in random numbers in Android

TreeSet myNumbers = new TreeSet();
Random randGen = new Random();

for (int i = 1; i <= 16; i++) {
    // number generation here
    int randNum = randGen.nextInt(16 - 1) + 1;
    for (;;) {
        if (myNumbers.add(randNum))
            break;
        else
            randNum = randGen.nextInt();

    }
    Toast.makeText(getApplicationContext(), "" + randNum, 100).show();
}

I want to generate random numbers between 1 and 16 and the same number should not be repeated.

The code above gives me output like:

2, 5, 7, 9, 1, 4, 10.4109446, -448831, 98824724, 11, 13, ...

I do not know why it gives me random numbers not in the 1-16 range, please help me out.

Upvotes: 0

Views: 522

Answers (3)

elias
elias

Reputation: 849

It's not a big issue, if you are working in the range 1-16, but your code results in rejection of some randomly drawn numbers, if they have already been picked before. In your solution, the expected value of nextInt() calls is proportional to n log(n), where n is the number of total elements you want to shuffle (16 in your case) – and the actual values can be much higher for a single run. You might consider using a more efficient implementation.

A solution which always uses only n calls:

ArrayList<Integer> originalNumbers = new ArrayList<Integer>();
Random randGen = new Random();
int max = 16;
for (int i = 1; i <= max; i++) {
    // initializing ordered list so it becomes 1, 2, ..., max
    originalNumbers.add(i);
}
for (int i = max; i >= 1; i--) {
    // picking a random number from the ordered list, and swapping it
    // with the last unpicked element which is placed closer to the
    // end of list, where the already picked numbers are stored
    int randNum = randGen.nextInt(i);
    Collections.swap(originalNumbers, i - 1, randNum);
    Toast.makeText(getApplicationContext(), "" + originalNumbers[i - 1], 100).show();
}

Upvotes: 1

Josh M
Josh M

Reputation: 11937

To generate a random number in a range, it is like:

int min = ...
int max = ...
int randNumber = min + new Random().nextInt(max - min + 1);

So in your example where you want to generate a random number from [1, 16], it would look like:

int randNumber = 1 + new Random().nextInt(16 - 1 + 1);

Or if you choose to simplify:

int randNumber = 1 + new Random().nextInt(16);

Also, you should really be using a while loop instead of an infinite for loop:

    final TreeSet<Integer> myNumbers = new TreeSet<>();
    final Random rand = new Random();
    for(int i = 0; i < 16; i++){
        int n = 1 + rand.nextInt(16);
        while(!myNumbers.add(n))
            n = 1 + rand.nextInt(16);
    }

Upvotes: 6

Jon Skeet
Jon Skeet

Reputation: 1500465

You're only generating one number in the range 1-15. You're then generating subsequent numbers with just nextInt:

if (myNumbers.add(randNum))
    break;
else
    randNum = randGen.nextInt();

That should be:

if (myNumbers.add(randNum))
    break;
else
    randNum = randGen.nextInt(16) + 1;

... and fix the initial call to nextInt to remove the "-1". (You don't need the 16 - 1, as explained in Josh's answer.)

Upvotes: 6

Related Questions