snippetcppMinor
Generate a random numbers class
Viewed 0 times
randomgenerateclassnumbers
Problem
I have made class to setup a random number bases on `
. It works fine, but I have doubt in std::default_random_engine mEngine{ std::random_device()() }; Did i implement it correctly? What is the best practice for random`?#include
#include
// for some compilers that's don't support nested templates with the same parameter
template
auto dist() -> typename std::enable_if::value, std::uniform_int_distribution >::type;
template
auto dist() -> typename std::enable_if::value, std::uniform_real_distribution >::type;
template
class Random
{
public:
Random(const T& min, const T& max)
: mUnifomDistribution(min, max)
{}
Random(const Random&) = delete;
Random(const Random&&) = delete;
Random& operator = (const Random&) = delete;
T operator()()
{
return mUnifomDistribution(mEngine);
}
private:
std::default_random_engine mEngine{ std::random_device()() }; //
//static auto dist() -> typename std::enable_if::value, std::uniform_int_distribution >::type;
//template
//static auto dist() -> typename std::enable_if::value, std::uniform_real_distribution >::type;
using type = decltype(dist());
type mUnifomDistribution;
};
int main()
{
::Random getRandom(0, 9);
for (int i = 0; i<9; ++i)
std::cout << getRandom() << '\n';
}Solution
I have a few comments on this.
Don't attempt to nest templates with the same parameter
First, as you've already discovered, nested templates with the same parameter are a violation of the C++ language standard, whether or not your compiler complains. (In fact, I would say it's broken if it does not complain.)
C++ standard, section 14.6.1:
A template-parameter shall not be redeclared within its scope (including nested scopes). A template-parameter shall not have the same name as the template name.
The definition of
The code doesn't need a
(Also note that the existing code misspells "Uniform")
The use of
The code in
Consider repeatability
To answer the question in your code, yes,
that line of code does seed
Now if you construct using the first version, you'll get the default seed each time. If you use the second constructor (with a dummy
Don't attempt to nest templates with the same parameter
First, as you've already discovered, nested templates with the same parameter are a violation of the C++ language standard, whether or not your compiler complains. (In fact, I would say it's broken if it does not complain.)
C++ standard, section 14.6.1:
A template-parameter shall not be redeclared within its scope (including nested scopes). A template-parameter shall not have the same name as the template name.
The definition of
mUniformDistribution can be simplerThe code doesn't need a
using statement. The declaration can be made directly:decltype(dist()) mUniformDistribution;(Also note that the existing code misspells "Uniform")
The use of
Random does not require a scopeThe code in
main refers to ::Random but that's really not necessary here. It should just be Random.Consider repeatability
To answer the question in your code, yes,
std::default_random_engine mEngine{ std::random_device()() };that line of code does seed
mEngine however, I'd suggest rethinking that feature. If you were using this random number generator for, say a simulation, you might want to be able to duplicate the simulation exactly and not simply always run it with new random numbers. To get the best of both worlds, you could keep your existing code, but eliminate the initialization of mEngine and add it instead to the constructors:Random(const T& min, const T& max)
: mEngine{},
mUniformDistribution(min, max)
{}
Random(const T& min, const T& max, bool )
: mEngine{ std::random_device()() },
mUniformDistribution(min, max)
{}
// ...
std::default_random_engine mEngine;Now if you construct using the first version, you'll get the default seed each time. If you use the second constructor (with a dummy
bool parameter) it will use a new seed each time.Code Snippets
decltype(dist<T>()) mUniformDistribution;std::default_random_engine mEngine{ std::random_device()() };Random(const T& min, const T& max)
: mEngine{},
mUniformDistribution(min, max)
{}
Random(const T& min, const T& max, bool )
: mEngine{ std::random_device()() },
mUniformDistribution(min, max)
{}
// ...
std::default_random_engine mEngine;Context
StackExchange Code Review Q#71313, answer score: 4
Revisions (0)
No revisions yet.