logic for implementing card shuffling

Question

i am trying to implement a class for a deck of cards. one of the function "deck" contains is shuffle.

void deck::shuffle()
{
    int cardsleft = deck::cards;
    for(int i = 0; i < cardsleft ; i++)
    {
        srand(time(NULL));
        int rdmn = rand() % cardsleft;
        card tempcard = deck::dcards[rdmn];
        deck::dcards[rdmn] = deck::dcards[i];
        deck::dcards[i] = tempcard;
    }

}

just to be clear, deck::cards is an integer counting no. of cards left in this deck, deck::dcards is an array of the cards in this deck.

MY QUESTION IS: this doesn't give very good results. Could it be I need to do this by pointers, passing the references? If so, how is it done?

Alternatively, could it be just bad algorithm? Perhaps I just need a better shuffling logic. Pray tell. Note: Yes I know there is a std::shuffle std::random_shuffle functions in the "algorithm.h" library, but these are only applicable to vectors, not arrays as I am using here, no?

UPDATE: nvm, i just realized my question can be answered here: http://blog.codinghorror.com/the-danger-of-naivete/

So it is a logic flaw, not my implementation.

Answer 1

If you need to shuffle a sequence you should consider std::random_shuffle (or std::shuffle in C++11). It reorders the elements in a sequence so that each possible permutation of those elements has equal probability of appearance. Yes, it can be used with arrays, not just vectors. Anything that is a "range".

Now going back to your code you have a problem with the seeding. The algorithms that generate (pseudo-)random numbers take an initial value, that is the seed. Giving the same seed they produce the same sequence of numbers. That means you only have to seed the random number generated once (per thread), otherwise you will produce the same sequence of numbers again and again. In this loop of yours you get the same number for a while, until the time changes an a new value for the seed is used.

Answer 2

When you do

srand(time(NULL));

you set the seed to the current time. Since this loop will run all inside one second (the normal resolution of the time function), the seed will be reset to the same starting value in the loop, causing you to almost always get the same random number every time.

You should seed the random-number generator only once, preferably as early as possible in the main function.

---

You might also want to check out the new PRNG functionality in C++11.