2

I have the following piece of code:

unsigned int randomInt()
{
    mt19937 mt_rand(time(0));
    return mt_rand();
};

If I call this code, for example 4000 times in a for loop, I don't get random unsigned integers, instead I get for example 1000 times one value and the next 1000 times I get the next value.

What am I doing wrong?

Baum mit Augen
  • 49,044
  • 25
  • 144
  • 182
marc3l
  • 2,525
  • 7
  • 34
  • 62
  • 9
    Print the value of the seed while you're at it. Also, get a coffee and think about what a "seed" is. – Kerrek SB Apr 09 '15 at 22:05
  • 1
    Everyone knows [this](https://xkcd.com/221/) is the right way to generate random numbers – Praetorian Apr 09 '15 at 22:07
  • 5
    I'm voting to close this question as off-topic because well it probably isn't but you haven't done the slightest bit of research into what the code you've written actually _does_, so... – Lightness Races in Orbit Apr 09 '15 at 22:12
  • 2
    We answer this on at least once a week: Take the initialization OUTSIDE the loop. I.e., `mt_rand(time(0))` exactly ONCE, then `mt_rand()` inside the loop. – Lee Daniel Crocker Apr 09 '15 at 22:14
  • @KerrekSB don't be mean, according to Bob Cecil Martin, the population of programmers doubles every 5 years. – Dmytro Aug 11 '17 at 16:57

2 Answers2

15

This happens because you call f 4000 times in a loop, which probably takes less than a mili second, so at each call time(0) returns the same value, hence initializes the pseudo-random generator with the same seed. The correct way is to initialize the seed once and for all, preferably via a std::random_device, like so:

#include <random>
#include <iostream>

static std::random_device rd; // random device engine, usually based on /dev/random on UNIX-like systems
// initialize Mersennes' twister using rd to generate the seed
static std::mt19937 rng{rd()}; 

int dice()
{
    static std::uniform_int_distribution<int> uid(1,6); // random dice
    return uid(rng); // use rng as a generator
}

int main()
{
    for(int i = 0; i < 10; ++i)
        std::cout << dice() << " ";   
}
vsoftco
  • 55,410
  • 12
  • 139
  • 252
0

A source of randomness is a resource that belongs to your entire program, not to a single function. You should pretty much never create a source of randomness inside a routine used to return a random value.

Good options include:

  • Pass a source of randomness into your function
  • Make your source of randomness a global variable
  • Make your source of randomness a static variable, so that initialization happens once.

One thing you might think to try that you should not do is to replace time(0) with a similar function that has a higher resolution; while you will get different results, this will still generate poor quality random numbers, and may even be much slower than generating random numbers properly. (I believe there are random number generators that can work properly with such usage, but those have to be designed for that purpose)

  • _"while you will get different results"_ Still almost certain not to be true. – Lightness Races in Orbit Apr 09 '15 at 22:19
  • @Lightning: I would expect (but haven't tried) `high_resolution_clock` to have good enough resolution to make a visible difference. But even if not, there are more extreme things that would work, like reading the time stamp counter. –  Apr 09 '15 at 22:23