17

I'm trying to implement a class that will serve as sort of a wrapper for the random library so I can use its objects and functions in (I think) a more intuitive way elsewhere in my code.

In my header I have something like this:

class RandomDevice{
private:
    unsigned long rand_seed;
    default_random_engine engine;
public:
    RandomDevice(unsigned long n);
    int randInt(int min, int max);};

And then in the .cpp file I implement those two functions (constructor and randInt) like so:

RandomDevice::RandomDevice(unsigned long n){
    rand_seed = n;
    default_random_engine engine;
    engine.seed(n);
}

int RandomDevice::randInt(int min, int max){
    uniform_int_distribution<int> distribution(min, max);
    return distribution(engine);
}

Finally, in my main.cpp I test these functions like this:

int main(){
    unsigned long seed = 1;
    RandomDevice my_rand(seed);

    cout << "random integer: " << my_rand.randInt(0, 10) << endl;
}

The problem is, no matter what I set the seed to in main.cpp, I always get the same values for my random numbers (not just randInt, I have other distributions as well). I've also tried setting seed to time(NULL) but the same problem occurs.

I'm really scratching my head at this one. Thanks!

InvisibleAndre
  • 181
  • 1
  • 1
  • 4

3 Answers3

15
default_random_engine engine;
engine.seed(n);

This is seeding the local engine, which is destroyed at the end of the constructor, not the class member engine, which ends up being default constructed.

Use the member initializer list instead:

RandomDevice::RandomDevice(unsigned long n) : rand_seed(n), engine(n){ }
T.C.
  • 133,968
  • 17
  • 288
  • 421
  • Yep, that did it! `default_random_engine new_engine;` `engine = new_engine;` `engine.seed(n);` worked perfectly. Thanks! – InvisibleAndre Feb 22 '15 at 01:09
  • 1
    @InvisibleAndre That's unnecessarily inefficient. See my edit. – T.C. Feb 22 '15 at 01:13
  • This is the best answer. There is no need to seed every time as proposed elsewhere. The engine is designed to be seeded once, just don't throw the engine away. – Alex Mar 04 '22 at 02:02
15

Try using std::chrono to feed the seed in each iteration or in every call. I think seeding time moments would give best randomness, since each time moment is a unique seed without any repetitions. I would do the following:

#include <chrono> 
...
default_random_engine engine; // or any other engine
engine.seed(std::chrono::system_clock::now().time_since_epoch().count());
Pavlos Sakoglou
  • 159
  • 1
  • 4
1

In your construction function, the new created local engine masks your private class memeber engine .
Just delete 2nd line of your RandomDevice::RandomDevice(), you will be fine.

Like this.

RandomDevice::RandomDevice(unsigned long n){
    rand_seed = n;
    engine.seed(n);
}

or using member intialize list as @T.C. said,

RandomDevice::RandomDevice(unsigned long n): rand_seed(n), engine(n) { }
Xin Cheng
  • 1,432
  • 10
  • 17