1

I'm looking for a way to generate two random numbers quickly in C++. The code is for a die and needs to generate a random number when rolled. My code currently looks like this:

Die::Die() {
    srand(time(NULL));
}

void Die::roll() {
    value = rand() % 6 + 1;
}

If I create two objects with this code, srand(), being a static (non-instance based) function, will generate a new seed for all objects. I've also tried doing this:

void Die::roll() {
    srand(time(NULL));
    value = rand() % 6 + 1;
}

Rather then having it in the constructor, but if I call them quickly like follows:

die0->roll();
die1->roll();

Their values are usually equal. Is there any way to actually make this random, every time? Thanks for all your help. :)

Mr Cherno
  • 315
  • 3
  • 10
  • 10
    You only need to seed the random number generator once. You can easily do this by moving the call `srand(time(NULL))` to the code that initialises your application – simonc Aug 19 '13 at 12:06
  • 2
    @simonc In fact, he SHOULD only seed it once. As it is now, if called several times in rapid succession, it will be reseeded with the same value over and over. – Viktor Dahl Aug 19 '13 at 12:08
  • You might want to look into the [new PRNG functionality of C++11](http://en.cppreference.com/w/cpp/numeric/random), especially the [`std::uniform_int_distribution`](http://en.cppreference.com/w/cpp/numeric/random/uniform_int_distribution) class and the example in that link. – Some programmer dude Aug 19 '13 at 12:09
  • 1
    @JoachimPileborg Yeah, it also avoids modulo bias. – Shafik Yaghmour Aug 19 '13 at 12:33

4 Answers4

4

Assuming the program is running quickly enough, the time (in seconds) will generally be the same, which means your seed will be the same, which means your random values with be the same. As the commenters have said, do the random seed elsewhere (like the start of your program or thread).

mark
  • 5,269
  • 2
  • 21
  • 34
  • If I do that, the values of the two Die instances are exactly the same, which is why I tried re-seeding it in the first place. – Mr Cherno Aug 19 '13 at 12:11
  • That won't be a problem; you don't need to reseed. – xen-0 Aug 19 '13 at 12:13
  • 1
    The two Die instances will be the same, but the random number generator will generate a pseudo-random string of numbers (the two Die will pull numbers from the same string of randoms). – mark Aug 19 '13 at 12:15
  • Yep, I figured it out, the issue was that I had the value variable at the top of the .cpp file rather than as a private variable in the header file; that's why the values were the same. I moved the srand() to the start of my program as well. Still getting used to OOP in C++. :) Thanks for all your help. – Mr Cherno Aug 19 '13 at 12:24
1

The problem is, if the srands occur close enough to each other in time, then you are seeding the rng with the same value, and so the first values you get after each seed will be the same. To avoid this, you can ensure that the seeding only occurs once. For example, you can remove the srand from your Die class, and seed sometime before your first use of Die.

Alternatively, if you don't want the consumer of Die to have the burden of calling srand, you could keep the srand in Die's constructor, but guard against calling it multiple times with a static bool.

Die::Die()
{
  static bool seeded = false;
  if (!seeded)
  {
    srand(time(NULL));
    seeded = true;
  }
}

Note that this srand can still affect other classes that srand() and rand(), but chances are this wouldn't matter. But if you need a random number within multiple classes that you are defining, you could look at creating an RNG singleton to generate numbers for you.

Kindread
  • 926
  • 4
  • 12
1

The least error prone method will be to use uniform_int_distribution from the random header. This also avoids modulo bias, this is basic example:

#include <iostream>
#include <random>

 class Die
 {
   public:
     template <class Generator>
     void roll( Generator &g )
     {
         std::uniform_int_distribution<int> dist(1,6);
         std::cout << dist(g) ;
     }

   private:

 } ;

int main()
{
    std::random_device rd;

    std::mt19937 e1(rd());

    Die d1, d2 ;

    for (int n = 0; n < 10; ++n) {
            d1.roll( e1 ) ;
            std::cout << " , " ;
            d2.roll( e1 ) ;
            std::cout << std::endl ;
    }
}

Alternatively, you can also use static members, as James suggests:

class Die
{
   public:
     Die()  {} ;

     void roll()
     {
         std::uniform_int_distribution<int> dist(1,6);
         std::cout << dist(e1) ;
     }

   private:
      static std::random_device rd;    
      static std::mt19937 e1 ;
} ;
Community
  • 1
  • 1
Shafik Yaghmour
  • 154,301
  • 39
  • 440
  • 740
  • Why be simple when one can be complicated, right? (Why should each instance of `Die` have its own generator, for example? And why pass the `Generator` as argument each time, rather than using a static member or a singleton? – James Kanze Aug 19 '13 at 13:25
  • @JamesKanze I meant to remove the second generator, the choice of passing the generator was arbitrary and I have added an example using static members. – Shafik Yaghmour Aug 19 '13 at 17:14
0

As others have said, you don't want to reseed the generator more than once. If Die is the sole user of rand (or even if it isn't), you can use a variable at name space scope to seed it:

int seeded = (srand( time( NULL ) ), rand());

Whether this is a good idea or not depends; it makes it impossible to duplicate a run of your code for debugging purposes, for example. If this is an issue, you'll want to 1) log the seed you use, and 2) add an option to the command line to specify the seed; the latter means calling srand from main.

(Note too that I've added a call to rand() in the initialization. In at least one wide spread implementation, the first call to rand() returns the seed, which means that it can be very predictable.)

James Kanze
  • 150,581
  • 18
  • 184
  • 329