HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Random Generation From Sequences

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
randomfromgenerationsequences

Problem

With the inclusion of ` 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:

  • 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 old typedef. Contrary to typedef, using can be templated, therefore it allows you to always be consistent, whether you have templates or not. Also, I find the X = Y syntax clearer and closer to variable assignment.



-
A tidbit:

std::is_same::value


I 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 time

I 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>::value

Context

StackExchange Code Review Q#26222, answer score: 6

Revisions (0)

No revisions yet.