patterncppModerate
Simple random generator
Viewed 0 times
randomsimplegenerator
Problem
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
#include
#include
#include
#include
template
auto randomEngine() -> typename std::enable_if_t
{
std::array 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
T random(T min, T max)
{
static_assert(std::is_integral::value || std::is_floating_point::value, "!");
using UniformInt = std::uniform_int_distribution;
using UniformReal = std::uniform_real_distribution;
using DistType = std::conditional_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) << ' ';
}Solution
Extra typenames
Every place you use
First of all,
And take it by reference:
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:
And alter
Every place you use
typename, it's unnecessary. std::enable_if_t will take care of that for you.thread_localFirst 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
auto randomEngine() -> std::enable_if_tAnd 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
typename Dist::result_type random(Dist& dist)
{
static auto& RandomEngine = randomEngine();
return dist(RandomEngine);
}And alter
dist to take arguments so that you could do:auto d = dist(0, 15);
for (auto i = 0u; i < 16; ++i)
std::cout << random(d) << ' ';Code Snippets
template<class T = std::mt19937, std::size_t N = T::state_size>
auto randomEngine() -> std::enable_if_t<!!N, T&>static auto& RandomEngine = randomEngine();template <class Dist>
typename Dist::result_type random(Dist& dist)
{
static auto& RandomEngine = randomEngine();
return dist(RandomEngine);
}auto d = dist<unsigned>(0, 15);
for (auto i = 0u; i < 16; ++i)
std::cout << random(d) << ' ';Context
StackExchange Code Review Q#114066, answer score: 10
Revisions (0)
No revisions yet.