Simple random generator

Question

Based on this question, I have made a simple program to print a sequence of random numbers. The program seems to run fine, but I would like to know if my implementation is correct and how I can improve it.

#include <random>
#include <array>
#include <algorithm>
#include <functional>
#include <iostream>
template<class T = std::mt19937, std::size_t N = T::state_size>
auto randomEngine() -> typename std::enable_if_t<!!N, T>
{
 std::array<typename T::result_type, N> seed_data;
 thread_local static std::random_device source;
 std::generate(std::begin(seed_data), std::end(seed_data), std::ref(source));
 std::seed_seq seeds(std::begin(seed_data), std::end(seed_data));
 thread_local static T seeded_engine(seeds);
 return seeded_engine;
}
template<typename T>
T random(T min, T max)
{
 static_assert(std::is_integral<T>::value || std::is_floating_point<T>::value, "!");
 using UniformInt = std::uniform_int_distribution<T>;
 using UniformReal = std::uniform_real_distribution<T>;
 using DistType = std::conditional_t<std::is_integral<T>::value, UniformInt, UniformReal>;
 static auto RandomEngine = randomEngine();
 DistType uniformDistribution(min, max);
 return uniformDistribution(RandomEngine);
}
int main()
{
 for (auto i = 0u; i < 16; ++i) 
 std::cout << random(0u, 15u) << ' ';
}

• Veedrac: Genuinely impressed that someone managed to seed a mt19937 correctly (I think), but it seems like such a waste of effort compared to just using a good RNG like from the PCG family, which also happens to handle much of this nonsense for you.

Answer 1

Extra typenames

Every place you use typename, it's unnecessary. std::enable_if_t will take care of that for you.

thread_local

First of all, thread_local implies static, so writing both is redundant. Secondly, if you're making it thread_local, that's effectively like making a thread-specific singlelton - so you should return a reference to it, not a copy:

template<class T = std::mt19937, std::size_t N = T::state_size>
auto randomEngine() -> std::enable_if_t<!!N, T&>

And take it by reference:

static auto& RandomEngine = randomEngine();

random()

I'm not sure it's a good idea to create the distribution every time. The distribution object itself can keep state, so I think it'd be a better idea to take it in instead:

template <class Dist>
typename Dist::result_type random(Dist& dist)
{
 static auto& RandomEngine = randomEngine();
 return dist(RandomEngine);
}

And alter dist<T> to take arguments so that you could do:

auto d = dist<unsigned>(0, 15);
for (auto i = 0u; i < 16; ++i) 
 std::cout << random(d) << ' ';