1

Possible Duplicate:
rand function returns same values when called within a single function c++

Why is rand() generating the same number?

die.h

#ifndef DIE_H
#define DIE_H


class Die
{
private:
    int number;
public:
    Die(){number=0;}
    void roll();
    int getNumber()const{return number;}
    void printValue();
};

#endif

die.cpp

#include"die.h"
#include<iostream>
#include<time.h>
using namespace std;

void Die::roll()
{
    srand(static_cast<int>(time(0)));
    number=1+rand()%6;
}

void Die::printValue()
{
    cout<<number<<endl;
}

main.cpp

#include"die.h"
#include<iostream>
using namespace std;

int main()
{
    Die d;
    d.roll();
    d.printValue();
    d.roll();
    d.printValue();
    d.roll();
    d.printValue();
}
Community
  • 1
  • 1
laura
  • 2,085
  • 13
  • 36
  • 68
  • 6
    Use `srand` once. It's currently seeding the same sequence over and over. – chris Nov 21 '12 at 19:18
  • Adding on to what @chris said, the key here is that `srand` is being called in rapid succession, so that `time` always returns the same value. Put some kind of sleep before the call to `srand` (for a second or so) and you'll see the behavior change. – John Dibling Nov 21 '12 at 19:28
  • You may want to take a look at this one: http://stackoverflow.com/questions/2254909/boost-random-number-generator – Sarang Nov 21 '12 at 19:29

3 Answers3

6

Your calls to die.roll() are so close together that time(0) is actually returning the same value every time, and, thus, your rand seed is the same for each call to .roll().

Try calling srand(static_cast<int>(time(0))); once (and only once) outside of .roll() (like in the Die constructor or main()).

2

You need to initialize the random generator with a "truly random" (or at least unique) seed, and do it only once.

This is usually done with srand(time(NULL)) at the beginning.

Numbers generated by rand() are not random, they are *pseudo*random: given the same seed, it will always return the same sequence. By default, I believe the initial seed is zero (however, it is for sure always the same between program runs - so without seeding you'll always get the same apparently random sequence every time).

LSerni
  • 55,617
  • 10
  • 65
  • 107
  • Sorry, guys. I got a cross-linked neuron with PHP :-( – LSerni Nov 21 '12 at 19:21
  • @chris, thanks for the vote of confidence, but mine was a genuine, no-excuse mistake. `now()` exists in too many dialects, and I should have paid more attention. – LSerni Nov 21 '12 at 19:23
  • Well, it does exist in C++11. It's just wrapped up in a couple of namespaces and classes :p – chris Nov 21 '12 at 19:24
0

As emartel pointed out, the only real change you need is to call time(NULL) instead of time(0).

Alternately you could call srand(time(NULL)) in the constructor and then just use rand() in roll().

John Dibling
  • 99,718
  • 31
  • 186
  • 324
Cargo23
  • 3,064
  • 16
  • 25
  • -1: That's not the (only) problem. – John Dibling Nov 21 '12 at 19:25
  • I'm not sure this -1 is warranted. According to the man page calling time with a non-null parameter causes the time to be stored in the memory address of the parameter. So I'm saying the thing that is really broken in the code is the call to time(0). And then, separately, they need to call srand once and rand multiple times. – Cargo23 Nov 21 '12 at 19:55
  • Well, the thing that is *really* broken (eg, the reason why OP is getting unexpected results) is that `time` is being called in rapid succession, hence returning the same values. However, I see where you're coming from now and I'll remove the downvote. The answer isn't wrong, largely because of the embedded "in the constructor", but that doesn't make it right. – John Dibling Nov 21 '12 at 20:03
  • @ers81239 in modern C++, NULL is literally another name for 0. Literal 0 where a pointer is expected (as in the argument to `time`) is the same as `NULL`. Even on oddball systems where the in-memory representation of a null-pointer is not all zero bits. It doesn't matter what happens when you call `time` with a non-null parameter because `time` isn't being called with a non-null parameter. – hobbs Nov 21 '12 at 20:07