patterncppMinor
Random Generation From Sequences
Viewed 0 times
randomfromgenerationsequences
Problem
With the inclusion of `
/*! \file intialized_generator.hpp
* \brief Helper struct to initialize and "warm up" a random number generator.
*
*/
#ifndef INITIALIZED_GENERATOR_SGL_HPP__
#define INITIALIZED_GENERATOR_SGL_HPP__
#include
#include
#include
#include
#include
namespace simplegl
{
namespace detail
{
template
struct initialized_generator
{
public:
typedef GeneratorType generator_type;
typedef typename generator_type::result_type result_type;
private:
generator_type generator_;
//! \brief Seeds the given generator with a given source of randomness.
/*!
* Some random number generators generate "poor" random numbers until their
* internal states are sufficiently "mixed up". This should seed the given
* generator with enough randomness to overcome the initial bad statistical
* properties that are seen in some of these generators.
*
* The code below was nabbed from
* http://stackoverflow.com/questions/15509270/does-stdmt19937-require-warmup
*
* \param rd A std::random_device, utilized to generate random seed data.
*/
void warmup(std::random_device& rd)
{
std::array seed_data;
std::generate_n(seed_data.data(), seed_data.s
into C++11, we can finally chuck out std::rand and start using generator with much better properties. Still, it's easy to get things wrong (uniform sampling where 0 occurs more than it should due to usage of % size comes to mind). To that end, I've written some wrapper libraries to ease usage of the library for some basic tasks, namely:
- Selection of values from a sequence with equal probability
- Selection of values from a sequence according to a weighted distribution
- Random "choice" -
k out of n selection.
The style is very similar to that of most of the algorithms in . Any comments or suggestions (or bug-finding) is welcome:
initialized_generator.hpp:
``/*! \file intialized_generator.hpp
* \brief Helper struct to initialize and "warm up" a random number generator.
*
*/
#ifndef INITIALIZED_GENERATOR_SGL_HPP__
#define INITIALIZED_GENERATOR_SGL_HPP__
#include
#include
#include
#include
#include
namespace simplegl
{
namespace detail
{
template
struct initialized_generator
{
public:
typedef GeneratorType generator_type;
typedef typename generator_type::result_type result_type;
private:
generator_type generator_;
//! \brief Seeds the given generator with a given source of randomness.
/*!
* Some random number generators generate "poor" random numbers until their
* internal states are sufficiently "mixed up". This should seed the given
* generator with enough randomness to overcome the initial bad statistical
* properties that are seen in some of these generators.
*
* The code below was nabbed from
* http://stackoverflow.com/questions/15509270/does-stdmt19937-require-warmup
*
* \param rd A std::random_device, utilized to generate random seed data.
*/
void warmup(std::random_device& rd)
{
std::array seed_data;
std::generate_n(seed_data.data(), seed_data.s
Solution
Overall, your code is clean, well written and seems very consistent (and the comments also seem relevant). There is not much to say, so I will focus on some details since I don't see anything else to review:
-
A tidbit:
I would replace
-
I would replace
As you can see, there is almost nothing to say about your current code. You even used
- First of all, Loki is right in his comment: your header guards names (
UNIFORM_SELECTION_SGL_HPP__,INTRUSIVE_RANDOM_CHOICE_SGL_HPP__...) are actually reserved to the implementation since they contain double underscores. You should remove the last underscore so that they are well-formed.
- It is mainly a matter of taste, but I would use the new type alias syntax (with
using) instead of the oldtypedef. Contrary totypedef,usingcan be templated, therefore it allows you to always be consistent, whether you have templates or not. Also, I find theX = Ysyntax clearer and closer to variable assignment.
-
A tidbit:
std::is_same::valueI would replace
GeneratorType by generator_type. It won't change the generated code but since you use the typedefs instead of the template parameters names in the rest of your code, it will be more consistent.-
//This branch will be optimized out at compile timeI would replace
will by should. While any decent compiler should optimize it, there is no such garantee that it will be the case for any compiler. Since you don't know which compiler your code will be compiled with, you shouldn't make the assumption (Ok, that's really a detail).- You could use curly braces instead of parenthesis in the initialization list of your constructors to prevent implicit type conversions.
As you can see, there is almost nothing to say about your current code. You even used
std::shuffle and that's great (even greater since std::random_shuffle will be deprecated in C++14). That's some really good code, kudos! :)Code Snippets
std::is_same<GeneratorType, std::mt19937>::valueContext
StackExchange Code Review Q#26222, answer score: 6
Revisions (0)
No revisions yet.