CODEWITHSUNDEEP
c++c++11randomshuffle
Szymon
Szymon

Reputation: 168

Default_random_engine passed into a function gives repeatable results

I have a class Permutation that inherits from std::vector<int>. I created a constructor that makes the object filled with non-repeating numbers. Randomness is meant to be guaranteed by <random> stuff, so the declaration goes like this:

/* Creates a random permutation of a given length
 * Input: n - length of permutation
 *        generator - engine that does the randomizing work */
Permutation(int n, default_random_engine generator);

Function itself looks like this (irrevelant details skipped):

Permutation::Permutation(int n, default_random_engine generator):
vector<int>(n, 0)
{
    vector<int> someIntermediateStep(n, 0);
    iota(someIntermediateStep.begin(), someIntermediateStep.end(), 0); //0, 1, 2...

    shuffle(someIntermediateStep.begin(), someIntermediateStep.end(), 
            generator);

    // etc.
 }

And is called in the following context:

auto seed = std::chrono::system_clock::now().time_since_epoch().count();
static std::default_random_engine generator(seed);

for (int i = 0; i < n; i++) 
    Permutation test(length, generator);

Code compiles perfectly fine, but all instances of Permutation are the same. How to force regular generation of random numbers? I know that default_random_engine should be binded to a distribution object, but hey, I don't have any – I use the engine only in shuffle() (at least at the moment).

Is there any solution or a workaround that still uses the goodness of <random>?

Upvotes: 0

Views: 1670

Answers (2)

Szymon
Szymon

Reputation: 168

So a childish mistake, just as I supposed – I mixed various solutions to similar problems in a wrong way.

As Benjamin pointed out, I mustn't copy the same engine over and over again, because it remains, well, the same. But this alone doesn't solve the issue, since the engine is pointlessly declared static (thanks, Zereges).

For the sake of clarity, corrected code looks like this:

Permutation(int n, default_random_engine &generator);

// [...]

Permutation::Permutation(int n, default_random_engine generator):
vector<int>(n, 0)
{
    vector<int> someIntermediateStep(n, 0);
    iota(someIntermediateStep.begin(), someIntermediateStep.end(), 0); //0, 1, 2...

    shuffle(someIntermediateStep.begin(), someIntermediateStep.end(), 
        generator);

    // etc.
}

// [...]
// some function
auto seed = chrono::system_clock::now().time_since_epoch().count();
default_random_engine generator(seed);

for (int i = 0; i < n; i++) 
    Permutation test(length, generator);

Upvotes: 0

Benjamin Lindley
Benjamin Lindley

Reputation: 103713

Your Permutation constructor takes the engine in by value. So, in this loop:

for (int i = 0; i < n; i++) 
    Permutation test(length, generator);

You are passing a copy of the same engine, in the same state, over and over. So you are of course getting the same results. Pass the engine by reference instead

Permutation::Permutation(int n, default_random_engine& generator)

That way its state will be modified by the call to std::shuffle.

Upvotes: 4

Related Questions