9

gcc 4.4.4 c89

I am using the code below. However, I keep getting the same number:

    size_t i = 0;

    for(i = 0; i < 3; i++) {
        /* Initialize random number */
        srand((unsigned int)time(NULL));
        /* Added random number (simulate seconds) */
        add((rand() % 30) + 1);
    }

I would like to get 0 to 30 returned. However, the last time I ran this I got 17 three times.

Many thanks,

ant2009
  • 27,094
  • 154
  • 411
  • 609
  • 2
    possible duplicate of [Random number function is misfiring](http://stackoverflow.com/questions/1068350/random-number-function-is-misfiring) – Pete Kirkham Jul 01 '10 at 16:24
  • Note: 0 to 30 is actually 31 unique values. You're code will always return 1 to 30. You should %31 (no "+ 1") if you really need 0-30. Off by 1 errors are so easy to miss. (apparently for 9 years in this case). – Dan Jun 27 '19 at 02:43

6 Answers6

22

You're seeding inside the loop (with the same value because of how quickly the loop will be executed), which causes the random number generated to be the same each time.

You need to move your seed function outside the loop:

/* Initialize random number */
srand((unsigned int)time(NULL));

for(i = 0; i < 3; i++) {
    /* Added random number (simulate seconds) */
    add((rand() % 30) + 1);
}
Mark Rushakoff
  • 249,864
  • 45
  • 407
  • 398
10

You need to call srand just once, at the beginning of your program.

srand initializes the pseudo random number generator using time in seconds. If you initialize it with a particular number, you will always get the same sequence of numbers. That's why you usually want to initialize it at the beginning using the time (so that the seed is different each time you run the program) and then use only rand to generate numbers which seem random.

In your case the time does not change from iteration to iteration, as its resolution is just 1 second, so you are always getting the first number of the pseudo-random sequence, which is always the same.

Michał Trybus
  • 11,526
  • 3
  • 30
  • 42
  • 1
    Is it really true that `need to call srand just once, at the beginning of your program.`? I mean I think that if one needs they should call `srand` whenever they need to change the random number generator's seed and to get a different sequence of random numbers? –  Jul 01 '10 at 16:27
  • It is true for this particular piece of code;). Of course, you can call it whenever you want to change the seed for some reason; it's not forbidden. – Michał Trybus Jul 01 '10 at 16:30
  • @skwllsp and that is usually only once _per request_! – alexanderpas Jul 01 '10 at 16:32
  • @skwllsp how is a different sequence achieved by setting a different seed better than a subsequence of the sequence generated using the old seed? – Michał Trybus Jul 01 '10 at 16:35
  • @Michał Trybus I was not talking about a better sequence. The main point was that I think it is not correct to say `once at the beginning of the program`. –  Jul 01 '10 at 16:42
  • @skwllsp true, strictly speaking it should be "need to call `srand` somewhere before the loop, not before every call to `rand`" – Michał Trybus Jul 01 '10 at 16:50
4

You need to do srand((unsigned int)time(NULL)) only once before the loop.

3

It is completely possible that the 3 times 17 are still completely random.

There is an about 1 in 10 chance of getting two numbers the same when using a range of 1-30 and three picks. (this is due to the birthday problem )

Now, getting three the same results has still a propability of 1 in 900 using the same range.

you might want to read more background on the analysis page of random.org

alexanderpas
  • 2,712
  • 2
  • 23
  • 33
  • In this case (even if it wasn't a coding mistake) getting two 17s in a row has nothing to do with the birthday problem. Making the assumption that each random number is independent, to calculate the probability of getting two 17s in a row would be 1/30*1/30 or 1/900. For three it would be 1/30*1/30*1/30, or 1/27000. – MarkD Jul 01 '10 at 17:21
  • but you forget there are 30 different possibilities of getting three the same numbers. 1/27000 * 30 = 1/900 – alexanderpas Jul 01 '10 at 22:39
  • now, for the two the same... there are actually three combination XYX, XXY and YXY with 30 posibilities each. 1/900 * 30 = 1/30 * 3 = 3/30 = 1/10 – alexanderpas Jul 01 '10 at 22:41
  • Remember, with the possibility of 365 options, and only 23 pulls, the chance of a duplicate is 50%! – alexanderpas Jul 01 '10 at 22:43
1

Seed to the pseudo Random number generator should be called only once outside the loop. Using time as a seed is good thing. However there is still a possiblity of getting the same random number.

Praveen S
  • 10,355
  • 2
  • 43
  • 69
1

I rather suggest also using gettimeofday() system call to retrieve the seed to be used to feed srand().

Something like


struct timeval tv;
...
gettimeofday(&tv, NULL);
srand(tv.tv_usec);
...

This approach can add more entropy in your pseudo number generation code. IMHO of course

Ciao ciao

Paolo Perego
  • 393
  • 3
  • 9