srand doesn't help me creating random numbers

Question

I'm trying to shuffle the cards in a deck. I create two numbers randomly and assign their values to eachother 100 times. But it doesn't create randomly. Weirdest part is this, I put a breakpoint to line "counter++;" to observe if the program really chooses different cards each time. When I'm debugging, if I click on the continue button too quickly it creates same index1 and same index2 every time(but index1 is not equal to index2). I thought it may be due to the time. (when I click too quickly on the continue button or when I don't put breakpoint at all, it creates same numbers due to the same time.) But if that's the case, why does the program creates different values for index1 and index2? There is no breakpoint between their lines. So the time must be very small. It creates different indexes though. I'm confused. How to get rid of?

Edit: Solution is to use srand for one time, in main.

void mix(string *a){
        srand(time(NULL));
        if (finisher == 100){
            return;
        }

        string b;
        int pos1;
        pos1 = rand() % 52;
        b = a[pos1];
        int pos2;
        pos2 = rand() % 52;
        cout << a[pos1] << "  " << a[pos2] << endl; //These lines are only for testing 
        a[pos1] = a[pos2]; 
        a[pos2] = b; 
        cout << a[pos1] << "  " << a[pos2] << endl; //These lines are only for testing 
        counter++;
        srand(time(NULL));
        mix(a);

    }

Also remember that srand does not generate trully random numbers as that is impossible on a computer, in the long term expect a tendency to get more lower numbers than high numbers. — rlam12, Dec 12 '15 at 15:08

score 4 · Answer 1 · edited May 23 '17 at 11:46

If you set a specific seed with srand, rand will generate a certain sequence of pseudo-random integers.
From man rand:

The srand() function sets its argument as the seed for a new sequence of pseudo-random integers to be returned by rand(). These sequences are repeatable by calling srand() with the same seed value.

time(NULL) will only change once per second, so you experience such a behavior.
Also, as @LightnessRacesinOrbit mentioned in some comments, you should call srand just once.

Since C++11, you have the <random> header. Use that instead.

score 3 · Answer 2 · edited Jun 20 '20 at 09:12

According to http://en.cppreference.com/w/cpp/numeric/random/srand

Notes:

Generally speaking, the pseudo-random number generator should only be seeded once, before any calls to rand(), and the start of the program. It should not be repeatedly seeded, or reseeded every time you wish to generate a new batch of pseudo-random numbers.

Standard practice is to use the result of a call to time(0) as the seed. However, time() returns a time_t value, and time_t is not guaranteed to be an integral type. In practice, though, every major implementation defines time_t to be an integral type, and this is also what POSIX requires.

By the way, why don't you use the C++11 random number generator which will produce random integer values i, uniformly distributed on the closed interval [a, b], that is, distributed according to the discrete probability function. See http://en.cppreference.com/w/cpp/numeric/random/uniform_int_distribution

Example code was provided is that cppreference page.

#include <random>
#include <iostream>
 
int main()
{
    std::random_device rd;
    std::mt19937 gen(rd());
    std::uniform_int_distribution<> dis(0, 51);
 
    for (int n=0; n<10; ++n)
        std::cout << dis(gen) << ' ';
    std::cout << '\n';
}

Rochet2 · Accepted Answer · 2015-12-12T14:49:40.013

You are likely having a problem with the call to srand. Your program runs the shuffle_cards function more than once within one second. This means that the timestamp you feed to srand in srand(time(NULL)); is the same on more than one call.

Two different initializations with the same seed will generate the same succession of results in subsequent calls to rand.

Thus both times the rand() call will generate the same index1 and index2 numbers as in the first call of the shuffle_cards function.

The solution is to not call srand or to call it once, for example when the program starts, or to call it less often.

You _should_ call srand but, yes, only once. – Lightness Races in Orbit Dec 12 '15 at 14:49 — Lightness Races in Orbit, Dec 12 '15 at 14:49

score 1 · Answer 4 · answered Dec 12 '15 at 16:11

As others have told you, part of the problem is all those calls to srand. Call it only once.

This isn't what you asked, but the code is not a good approach to shuffling a deck. First, instead of calling itself, shuffle should be written with a loop:

void shuffle(string* a) {
    for (int i = 0; i < 100; ++i) {
        int index1 = std::rand() % 52;
        int index2 = std::rand() % 52;
        string temp = a[index1];
        a[index1] = a[index2];
        a[index2] = temp;
    }
}

Note that this code uses initialization instead of defining an uninitialized variable and then assigning a value to it. Also, I've rearranged the order to make it clearer what's going on.

But this is still not a good way to shuffle cards; it will typically leave much of the deck unchanged. This is a well understood problem. Read about the Fisher-Yates shuffle:

void shuffle(string* a) {
    for (int i = 0; i < 51; ++i) {
        int idx = std::rand() % (52 - i) + i;
        string temp = a[idx];
        a[idx] = a[i];
        a[i] = temp;
    }
}

This can be improved by using the standard library's swap function:

void shuffle(string* a) {
    for (int i = 0; i < 51; ++i) {
        int idx = std::rand() % (52 - i) + i;
        std::swap(a[i], a[idx];
    }
}

And, unless you're doing this as a learning exercise, you can get rid of all that code:

void shuffle(string* a) {
    std::random_shuffle(a, a + 52);
}

Note that I've assumed that string here is some funky typedef for char or something char-like, and not std::string, for which none of this code would work sensibly.

srand doesn't help me creating random numbers

4 Answers4