Is it correct random generator function?

Question

I use this function to generate random integer.

int rnd(int min, int max)
{
    static int srand(time(0));
    return min + rand() % max;
}

Is it right? May be better to move srand(time(0)); to the main function? Like this:

int rnd(int min, int max){
    return min + rand() % max;
}
int main(){
srand(time(0));
..............
}

It's either way ok, in later case, srand may be unnecessarily intialized even you don't call rnd at all, to be precise use (max - min + 1) in denominator — Dr. Debasish Jana, May 27 '14 at 13:10
@πάνταῥεῖ, what is wrong here? A static data gets initialized when the encompassing function is called — Dr. Debasish Jana, May 27 '14 at 13:15
BTW, library random (in C++) provide cleaner way to generate random number (as `std::uniform_int_distribution`). — Jarod42, May 27 '14 at 13:15
the static init is wrong. It could be `static int somevar = srand(time(0));` but only if srand returned some dummy value. — Bgie, May 27 '14 at 13:18

score 3 · Answer 1 · answered May 27 '14 at 13:11

Definitely. You are now not calling the function but creating an integer variable named srand instead which shadows the global srand().

It does not help to re-seed the random generator each time -- it would even return always the same value if called during the same second! Calling it just once in main() would be better.

Also it should probably be min + rand() % (max - min + 1).

score 3 · Accepted Answer · answered May 27 '14 at 13:19

Your first version does not do what you intend:

int rnd(int min, int max)
{
    static int srand(time(0));
    return min + rand() % max;
}

This does not call the function srand. It declares a static variable of type int and assigns it the value of time(0). Like int a(42); (or in C++11 int a{42};) declares an integer and sets it to 42.

Definitely use the second variant. It also makes live much easier, if you want to have another function for creating random values in a different way.

Additionally I strongly recommend to use std::random (or boost::random if you are not allowed to use C++11 for some obscure reason).

+1 for mentioning ``. `rand()` is discouraged nowadays. – Chnossos May 27 '14 at 13:29 — Chnossos, May 27 '14 at 13:29

score 1 · Answer 3 · answered May 27 '14 at 13:13

In general you only need to initialize the pseudorandom number generator once, so in the main function is a good idea.

I believe Netscape once had a bug due to seeding the random number generator too often, which caused its SSL implementation to be more easily cracked.

score 1 · Answer 4 · answered May 27 '14 at 13:23

Is it right?

If by that you mean "is it generating a random number?", the answer is yes.

If you mean "is it optimal?" then the answer is no. You only need to initialize the sequence of random number once.

May be better to move srand(time(0)); to the main function

Maybe ... (you know what you need better than us).

What are you trying to achieve (that is, what are you using this random generator for)? If it is anything with financial data, online gambling application (or anything that takes/manages/costs money) it is not ok (nor is it to use srand and rand - you need something stronger).

Is it correct random generator function?

4 Answers4