Use of static in myRandom::generateRandomInt causes faulty number generation

[Bakkes]

I enjoyed the video and decided to check out the repo, and saw that generateRandomInt probably does not do what you intend it to do.

The use of "static" for the distribution causes the distribution to be initialized only once, so whatever the first call to generateRandomInt is will determine which numbers get generated every time thereafter.

See:

Papy/src/myRandom.cpp

Line 16 in e6e6b30

static std::uniform_int_distribution<> distrib(min, max);

For example:

#include <random>
#include <iostream>

thread_local std::random_device rd;
thread_local std::mt19937 gen(rd());

int generateRandomInt(int min, int max) {
    static std::uniform_int_distribution<> distrib(min, max);
    return distrib(gen);
}

int main()
{
    generateRandomInt(0, 5);
    for (int i = 0; i < 100; ++i)
    {
        std::cout << generateRandomInt(0, 100) << '\n';
    }
}

Will print 100 numbers between 0 and 5, instead of numbers between 0 and 100.
As far as I know these distribution classes are fairly lightweight and will probably be inlined by the compiler anyway, so marking them static will not have any big performance improvements I think, But if they do and you want a distribution that's marked static, you could always have a templated function if you know the range of values at compile time.

template<int MIN, int MAX>
int generateRandomInt() {
    static std::uniform_int_distribution<> distrib(MIN, MAX);
    return distrib(gen);
}

int main()
{
    generateRandomInt<0, 5>();
    for (int i = 0; i < 100; ++i)
    {
        std::cout << generateRandomInt<0, 100>() << '\n'; //Does print values between 0-100
    }
}

[korokuchum]

Uhhhh. No, the templates is not the way to fix it.
Because now you have a bunch of copies of the same method in your final binary.
And what if you can't select min/max at compile time?
For example if user provides min max via console input.

Just don't put "static" at all.
Under the hood uniform_int_distribution is just a pair of ints.
It is blazingly fast to initialize and better yet compiler will optimize it further.