1

Im trying to shuffle a deck of cards but random_shuffle produces the same result every time

void
Deck::shuffle() {

    cout << "SHUFFLING CARDS!!!\n\n";

    srand(time(0));
    random_shuffle(cards.begin(), cards.end());

    displayCards();
}
blitzeus
  • 485
  • 2
  • 10
  • 28

5 Answers5

5

That's because you seed pseudo-random number generator to the same value each time:

srand(time(0));

The granularity of time are seconds, if I'm not mistaken. If you pause the execution for some time between calls to Deck::shuffle() you should see different results.

Remove that line from the function and call it once at the start of your program.

jrok
  • 54,456
  • 9
  • 109
  • 141
1

I think that the problem is because you are putting srand(...) inside of your function.

Try to move it outside (so that it will be executed only once)

Salvador Dali
  • 214,103
  • 147
  • 703
  • 753
1

You are reseeding the random number generator each time you call shuffle.

You should only seed the random number generator once per application (typically in your application initialization):

int main()
{
    // other initialization
    srand(time(NULL)); // seed the number generator

    // ...
}
Zac Howland
  • 15,777
  • 1
  • 26
  • 42
0

It is important to know that to be able to receive a "random" number you have to seed the generator. Also it should be seeded outside the function.

srand(time(NULL));

The use of the time function will help ensure that you will receive a random number.

time.h does need to be included for it to work. For more reference on srand, click here.

Hashman
  • 151
  • 5
0

I'm not familiar with random_shuffle, but here's a function that does a perfect shuffle - in other words, each 52! permutations of the deck has to be equally likely.

This is from Gayle Laakmann's Cracking the Coding Interview (Question 20.2)

void Deck::shuffle() {

int temp, index;

for (int i = 0; i < cards.size(); i++){

    index = (int) (rand() %(cards.size() - i)) + i;

    temp = cards[i];
    cards[i] = cards[index];
    cards[index] = temp;

        }
}
P0W
  • 46,614
  • 9
  • 72
  • 119
  • 3
    That's exactly what `random_shuffle` does (although it doesn't require the type to be copyable). – Mike Seymour Aug 16 '13 at 17:29
  • I was going to protest that this will choose the first 16 cards more often, making the last 36 slightly less shuffled, but upon review, I'm not certain that would actually make any permutations less likely than another, so this may very well be equal. Hard to say. – Mooing Duck Aug 16 '13 at 17:35
  • 1
    I just looked in header, its as: `for (RandomAccessIterator it = first + 1; it != last; ++it) std::iter_swap(it, first + (std::rand() % ((it - first) + 1)));` – P0W Aug 16 '13 at 17:42
  • @ZacHowland yeah, but there's an implementation using a custom generator `gen` – P0W Aug 16 '13 at 17:47
  • 2
    @MooingDuck: Indeed, there will be a slight bias (on the order of `1/RAND_MAX`) towards the first elements remaining near the front. But if that's a concern, then you should be using something better than `rand()` anyway. – Mike Seymour Aug 16 '13 at 17:48
  • 1
    @POW: In your post you said you were not familiar with random_shuffle. I was just providing the link for reference. It is a pre-written (and well tested) algorithm that does exactly what your posted code does (and you can customized it if you like). – Zac Howland Aug 16 '13 at 17:51