I have made class to setup a random number bases on <random>
. 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 <random>
#include <iostream>
// for some compilers that's don't support nested templates with the same parameter
template <typename T>
auto dist() -> typename std::enable_if<std::is_integral<T>::value, std::uniform_int_distribution<T> >::type;
template <typename T>
auto dist() -> typename std::enable_if<std::is_floating_point<T>::value, std::uniform_real_distribution<T> >::type;
template<typename T>
class Random
{
public:
Random(const T& min, const T& max)
: mUnifomDistribution(min, max)
{}
Random(const Random<T>&) = delete;
Random(const Random<T>&&) = delete;
Random<T>& operator = (const Random<T>&) = delete;
T operator()()
{
return mUnifomDistribution(mEngine);
}
private:
std::default_random_engine mEngine{ std::random_device()() }; // <-- here the doubt - is it seeding?
//template <typename T>
//static auto dist() -> typename std::enable_if<std::is_integral<T>::value, std::uniform_int_distribution<T> >::type;
//template <typename T>
//static auto dist() -> typename std::enable_if<std::is_floating_point<T>::value, std::uniform_real_distribution<T> >::type;
using type = decltype(dist<T>());
type mUnifomDistribution;
};
int main()
{
::Random<int> getRandom(0, 9);
for (int i = 0; i<9; ++i)
std::cout << getRandom() << '\n';
}
1 Answer 1
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 mUniformDistribution
can be simpler
The code doesn't need a using
statement. The declaration can be made directly:
decltype(dist<T>()) mUniformDistribution;
(Also note that the existing code misspells "Uniform")
The use of Random
does not require a scope
The 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.
-
\$\begingroup\$ I dont understand the use of
dist<T>()
. Is it a function, and if so why is it able to be called? It has a non-void return type, but was not defined with a body to return a value? Does the use ofdecltype
just take the return type and bypass the body? \$\endgroup\$flakes– flakes2014年12月02日 14:47:58 +00:00Commented Dec 2, 2014 at 14:47 -
\$\begingroup\$ @Calpratt ... this machinsim known as SFINAE. check this link for more detail en.cppreference.com/w/cpp/language/sfinae \$\endgroup\$MORTAL– MORTAL2014年12月02日 15:23:21 +00:00Commented Dec 2, 2014 at 15:23
<random>
or not \$\endgroup\$> >::type;
. i updated the code. thank you. \$\endgroup\$error: shadows template parm ‘class T’ template<typename T>
\$\endgroup\$