Optimal random integer function

Question

What I have

Header file:

int randomInt(int start, int end);

Implementation file:

int randomInt(int start, int end) {
 static std::random_device rd; // Obtain a random number from hardware
 static std::mt19937 eng(rd());// Seed the generator
 std::uniform_int_distribution<> dist(start, end);
 return dist(eng);
}

What I want to do

Would it be optimal to make this function inline or anything else? If so, how would I deal with the static variables?

Answer 1

Putting the ideas of your code and the remarks from @JDługosz. He's right about not wanting to recreate the distribution over and over again, that needs to be initialized only once to guarantee a good distribution, and this example also avoids calling the threadsafety blocks for statics by initializing a reference to the engine in the constructor. All in all it will look like this :

// header file
#include <random>
namespace random
{
 auto& get_engine()
 {
 static std::random_device rd;
 static std::mt19937 eng{ rd() };
 return eng;
 }
 template<typename type_t>
 struct generator_t 
 {
 generator_t(const type_t& start, const type_t& end) :
 m_distribution{ start,end },
 m_engine{ random::get_engine() } // initialize reference to engine from static (performance optimization)
 {
 }
 inline type_t operator()()
 {
 return m_distribution(m_engine);
 }
 private:
 std::uniform_int_distribution<type_t> m_distribution;
 std::mt19937& m_engine;
 };
}
// main file starts here :
#include <iostream>
int main()
{
 random::generator_t<int> random_generator(1,10);
 for (std::size_t n = 0; n < 20; ++n)
 {
 auto value = random_generator();
 std::cout << value << "\n";
 }
 return 0;
}

• Toby Speight: If you write auto random_generator = random::generator_t(1,10);, then deduction will yield a random::generator_t<int> without the need to specify the template type. It probably makes sense for operator() to be declared const, as it doesn't change the logical state of the generator. I'm pretty sure the distribution member can be const, too.
• Toby Speight: I'm not convinced that having all instances reference the same static std::random_device object is any better for multi-thread operation than the static variable in the original code.
• Pepijn Kramer: @TobySpeight Yes the compiler can auto deduce it (I just left it in for clarity in the example). About const : yes that should have been added. And yes the reference is a single-threaded optimzation :)

Answer 2

If you made it inline and defined it in a header, the static variables would work just the same.

It would be more efficient to not use local static variables because it requires thread-safe checks each time it is called.

When you use random numbers, you often need more than one in the same range. So creating the uniform distribution object once and using it many times is more optimal. You can also specify a different range on an existing object, without constructing and destroying it on every call; but I don't know how that compares speed-wise.

You might want to make it a template, so it can handle different sizes of ints.