Repeated and Predictable RNG Values in C

Question

I'm trying to generate 10,000 random numbers in a row in C and am having trouble getting random or even randomish results using the pseudo RNG. I used modulus in a way that I think should create uniformity, which it does, but the results are equivalent to 0,0,0,0,0,0,0,0,0,1,1,1,1,1,1,1,1,1,2,2,2,2,2,2,2,2,3,3,3,3,3,3,3 etc. when run in a loop in another function calling RNG(4).

int RNG(int n) {
    int range = RAND_MAX - (RAND_MAX % n);
    srand(time(NULL));
    int x = rand();
    while (x > range) {
        x = rand();
    }
    return x % n;
}

Any way to get it closer to 1,3,2,0,2,3,1,0,0,3,2,0,1 etc. would be appreciated!

Thank you!

EDIT: Thanks for the responses everyone! Moved the seeding to the start of the function calling RNG and everything is dandy now!

1. Choose whether you use `C` or `C++`. 2. If you've chosen `C++`, I have [some good news for you](http://en.cppreference.com/w/cpp/numeric/random/uniform_int_distribution) — Fureeish, Jun 10 '18 at 23:42
You are resetting the seed using `srand` every time you call `RNG`... — CAFxX, Jun 10 '18 at 23:50
If you want a *repeatable* sequence of pseudo-random numbers, call `srand()` (*once*) with a constant value, not with `time(NULL)`, which is designed to give you a new sequence every time it's run. Or don't all `srand()` at all, which is equivalent to calling `srand(1)`. — Keith Thompson, Jun 11 '18 at 00:08

Eric Postpischil · Accepted Answer · 2018-06-10T23:59:07.650

Do not call srand every time you want to generate a number. srand initializes the pseudo-random number generator and is intended to be called just once at the start of your program, or when you want to reset the generator. By resetting it every time, you are forcing rand to generate the same numbers every time you call it within each second on the clock.

Do not use x % n to reduce the number to a desired range. Old implementations of rand are notoriously bad and have patterns in the low bits. Instead, use x / ((RAND_MAX+1u) / n).

The code int range = RAND_MAX - (RAND_MAX % n); is flawed. Suppose n is 4 and RAND_MAX is 7, meaning rand returns 0 to 7. This code sets range to 4, and then while (x > range) x = rand(); discards 5, 6, and 7, while it retains 4. There are two bugs here: The code keeps the five values 0, 1, 2, 3, and 4, which is a mismatch to (not a multiple of) the desired range of 4, and it unnecessarily discards values. If we had kept 4, 5, 6, and 7, we would have a match. You could use:

unsigned range = (RAND_MAX + 1u) - ((RAND_MAX + 1u) % n);

and:

while (x >= range) x = rand();

If you are using C++, switch to using std::uniform_int_distribution. If you are using C, check the quality of rand in your implementation or switch to another generator such as the POSIX srandom and random.

I'm going to be snarky but... I guess this warrants a mention of https://xkcd.com/221/ — CAFxX, Jun 10 '18 at 23:52

score 0 · Answer 2 · answered Jun 10 '18 at 23:56

As noted elsewhere, the fix to the repeated numbers is to move the call to srand(time(NULL)) outside this function and call it only once per program at the beginning.

As for why you're getting repeated numbers: The function is being called several times per second. Each time the function executes in a given second, time(NULL) returns the same number, which this code uses to seed the random number generator.

The sequence of random numbers generated from a particular seed will always be the same. This code takes the first number from that sequence, which is always the same for one second, until time(NULL) returns a new value.

Repeated and Predictable RNG Values in C

2 Answers2