What's wrong with my code? Generate Unique Random Number

Question

I'm trying to generate 10 unique random numbers between 1 and 10. I keep getting duplicate numbers. Can someone tell me what the problem is? What am I missing or need to fix in my code? Thank you!

students[0].id = rand() % 10 + 1;
for (int i = 1; i < 10; i++)
{
    students[i].id = rand() % 10 + 1;
    for (int j = 0; j < i; j++)
    {
        if (students[i].id == students[j].id)
        {
            students[i].id = rand() % 10 + 1 ;
        }
    }
}

for (int i = 0; i < 10; i++)
{
    printf("%d\n", students[i].id);
}

A random number is never guaranteed to be unique. You will have to think of a better idea :) — leppie, Apr 11 '13 at 05:45
You should make a set of 10 numbers, and remove the next one picked (using rand) from the set, or keep calling rand until you get one that still remains in the set (then remove it). — GalacticJello, Apr 11 '13 at 05:55
Use this formula to generate random number [M + rand() / (RAND_MAX / (N - M + 1) + 1)](http://c-faq.com/lib/randrange.html) — Grijesh Chauhan, Apr 11 '13 at 05:59

score 2 · Answer 1 · answered Apr 11 '13 at 05:45

2

The function rand() isn't guaranteed to generate unique random numbers. Moreover, your way of limiting the range (mod 10) is especially bad and is likely to generate many duplicates.

The simplest way to generate your sequence is probably to shuffle an array of 10 values from 1 to 10.

answered Apr 11 '13 at 05:45

cnicutar

178,505
25
365
392

1

Agreed, though it should be made clear that the numbers in the array will not be RANDOM, they will be in a RANDOM order – J.Miller Apr 11 '13 at 05:48

score 2 · Accepted Answer · answered Apr 11 '13 at 05:51

if (students[i].id == students[j].id)
        {
            students[i].id = rand() % 10 + 1 ;
        }

In this line , you may get duplicate.

if students[i].id & students[j].id = 5 means it will get true. But in this line students[i].id = rand() % 10 + 1 ; ,you may get again 5

Instead of above line, you may use this code.

students[0].id = rand() % 10 + 1;
for (int i = 1; i < 10; i++)
{
    students[i].id = rand() % 10 + 1;
    for (int j = 0; j < i; j++)
    {
        if (students[i].id == students[j].id)
        {
           i--;
           break;
        }
    }
}

score 1 · Answer 3 · edited May 23 '17 at 11:57

1

rand() is random in nature, so there is no guarantee to give you unique results. You need to do something like this - keep track of all the numbers encountered so far and call the rand() till you find unique numbers

edited May 23 '17 at 11:57

Community

1
1

answered Apr 11 '13 at 05:45

karthikr

97,368
26
197
188

score 0 · Answer 4 · answered Apr 11 '13 at 05:45

0

Random does not mean "No Duplicates".

answered Apr 11 '13 at 05:45

J.Miller

477
1
3
14

score 0 · Answer 5 · answered Apr 11 '13 at 05:47

0

After you entered this if clause if (students[i].id == students[j].id) and modified students[i].id, you need to check for duplicates again.

answered Apr 11 '13 at 05:47

Chiara Hsieh

3,273
23
32

score 0 · Answer 6 · answered Apr 11 '13 at 05:59

Try This One: replace if with while. I hope you get your answer.

students[0].id = rand() % 10 + 1;

for (int i = 1; i < 10; i++)

{

students[i].id = rand() % 10 + 1;
for (int j = 0; j < i; j++)
{
    while (students[i].id == students[j].id)
    {
        students[i].id = rand() % 10 + 1 ;
    }
}

}

for (int i = 0; i < 10; i++)

{

printf("%d\n", students[i].id);

}

score 0 · Answer 7 · answered Apr 11 '13 at 06:09

0

I think you want to sample without replacement. You can store the indices in an array, randomly pick up one & simultaneously remove it from the array.

So that when you draw next time, it doesn't repeat.

answered Apr 11 '13 at 06:09

Nishanth

6,932
5
26
38

score 0 · Answer 8 · 2013-04-11T06:19:01.073

0

You need recursion.

public static void main(String[] args)
{
    System.out.println(generateRandomNumbers(10, new ArrayList<Integer>()));
}

private static List<Integer> generateRandomNumbers(Integer maxLimit, List<Integer>    randomNumberList)
{
    for (int i = 0; i < maxLimit; i++)
    {
        Integer tempRandom = new Random().nextInt(10);
        if (randomNumberList.contains(tempRandom))
            generateRandomNumbers(1, randomNumberList);
        else
            randomNumberList.add(tempRandom);
    }
    return randomNumberList;
}

edited Apr 11 '13 at 06:19

answered Apr 11 '13 at 06:11

You're answering a C question with Java code, which uses recursion? What justification do you have that this unbound recursive function won't invoke undefined behaviour by proxy of a "stack overflow"? – autistic Apr 11 '13 at 06:51
Does it really matter? I've just wanted to show the algorithm. With C, there will be a function, and it'll call itself when generated random number exists in generated number list already. BTW, i don't guarentee the possibility of exception or error. – Apr 11 '13 at 08:10
There are no "exceptions" in C, and C has "undefined behaviour". Code that invokes undefined behaviour may appear to function as desired on your system, while failing on other systems in subtle or devastating ways. Perhaps a segfault will be thrown if someone causes input that overflows the stack, or perhaps it'll be an opportunity for them to maliciously compromise the system... – autistic Apr 11 '13 at 09:11
I said i don't guarentee the possibility of exception **or error**. It was an example for the question. I didn't mean that i solved the question. You know C, you are the champion, solve it if you can. – Apr 11 '13 at 10:38

score 0 · Answer 9 · answered Apr 11 '13 at 06:46

What happens if RAND_MAX (commonly 32767) isn't evenly divisible by 10? You're likely to get values between 1 and 7 more often than 8 and 0. That's a bias.

I would suggest discarding any values greater than or equal to 32760 (or rather, RAND_MAX - RAND_MAX % 10), and using the division operator to construct your random number:

int x;
do {
    x = rand();
} while (x >= RAND_MAX - RAND_MAX % 10);
x /= RAND_MAX / 10;

You'll see a fairly drastic improvements from this; In fact, that seems like your most significant bias. However, the distribution of the values you get still isn't required to be uniform. Use a lookup table to discard any values you've previously selected:

int selected[10] = { 0 };
for (int i = 0; i < 10; i++) {
    int x;
    do {
        x = rand();
    } while (selected[x / RAND_MAX / 10] || x >= RAND_MAX - RAND_MAX % 10);
    x /= RAND_MAX / 10;
    selected[x] = 1;

    student[i].id = x;
}

for (int i = 0; i < 10; i++) {
    printf("student[%d].id: %d\n", i, students[i].id);
}

What's wrong with my code? Generate Unique Random Number

9 Answers9