0

I'm having trouble with a random generator. I'm trying to print out random values and I'm getting almost the same value every single time.

This is what I have:

void Deck::shuffle() {
    StackNode<Card>* top = stack->top;

    for (int i = 0; i < stack->numNodes - 1; i++) {
        int x = random(i);
        StackNode<Card>* temp = findCard(x);
        //cout << "Random index was: " << random(i) << endl;
        //cout << "Face value of random was: " << temp->data.getFaceVal() << endl;
        cout << "Top: " << top->data.getFaceVal() << endl;
        cout << "Temp: " << temp->data.getFaceVal() << endl;
        swapX(top,temp);
    }
}

Here's my random generator function:

int random(int index) {
    int r;
    srand(time(NULL));
    cout << "Index: " << index << endl;
    r = rand() % 50;
    cout << "Random value: " << r << endl;
    return r;
}
antonpp
  • 2,333
  • 23
  • 28
  • 4
    creating rand() instance too quickly: create once, use many... – Mitch Wheat Apr 21 '15 at 01:10
  • @MitchWheat I thought the calls produced different values every time? What do I need to change? The portion in my random function? Or in my loop in the shuffle function? –  Apr 21 '15 at 01:11
  • 2
    `srand()` seeds the generator and should be called only once at the start of the program. Rapidly seeding it with the time cause it to receive the same seed over and over again because `time()` is only accurate to the nearest second. And the computer can do millions of things every second before the clock changes. – Galik Apr 21 '15 at 01:13
  • 1
    the most recommended way in modern C++ (i.e. C++11/14) is to use `` and the classes/functions within, like a combination between a random engine and a distribution, see an example [here](http://en.cppreference.com/w/cpp/numeric/random/uniform_int_distribution). `rand/srand` will probably be deprecated in the future C++17 standard. – vsoftco Apr 21 '15 at 01:15
  • @Galik Okay. I'm sorry to ask, but do you mean I need to move srand() and only have my random function use "r = rand() % 50"? –  Apr 21 '15 at 01:17
  • @vsoftco I really appreciate that reference. I might use that if I cannot get the current implementation to work –  Apr 21 '15 at 01:18
  • @aaaa `srand` seeds the random number generator. From a given seed, a random number generator generates the same sequence, every time is run. So you need to seed only once, at the beginning of the program. If you seed twice for example with the same seed (this happens in your case, since between the rapidly succeeding runs `time(NULL)` returns the same seed), the second time you'll get the same numbers. Just realized I repeated what Galik wrote above... – vsoftco Apr 21 '15 at 01:20
  • @vsoftco Okay, I think I have it clear now. –  Apr 21 '15 at 01:28

2 Answers2

2

I think you can use std::shuffle here for your problem. Like this:

   #include <vector>
   #include <algorithm>

    void Deck::shuffle() {
        StackNode<Card>* top = stack->top;

        std::vector<StackNode<Card>*> cards;

        for (int i = 0; i < stack->numNodes - 1; i++) {
            cards.push_back(findCard(i))
        }

        std::shuffle(cards.begin(), cards.end());

        for (auto card : cards) {
            std::cout << card->data.getFaceVal() << std::endl;
        }
    }

By the way, I would recommend you to call srand only once in your code.

antonpp
  • 2,333
  • 23
  • 28
  • did you mean call srand only once? – DaveB Apr 21 '15 at 01:31
  • I really appreciate this. I'm not very familiar with STL, and usually shy away from it, but I appreciate the time into this. Unfortunately my teacher is forcing us to write our own shuffle and not use the STL one. I didn't specify that, but probably should have. @Anton –  Apr 21 '15 at 01:31
  • @DaveB yeah, I wasn't sure where to call srand() once. but I think I got it. Thank you all so much. Is there a way to upvote multiple answers? you all helped me understand it better –  Apr 21 '15 at 01:32
  • @aaaa you cannot yet vote on the answers, as you don't have enough reputation. You can (and should) however accept an answer that you believe addresses your issue. – vsoftco Apr 21 '15 at 01:34
  • @vsoftco Okay. Where do I go to accept an answer? wait nvm found it. i chose his because he explicitly stated where to put the line, but I found all the posts helpful. Im not trying to exclude :( thank you all for the help –  Apr 21 '15 at 01:38
1

rand() is a pseudo random number generator. The numbers it generates appear to be random, but they are generated by a completely deterministic function. The seed that you give it with sand() determines the starting point for the function. If you give it the same seed it will generate the same sequence of random numbers. You can try this and see for your self by seeding with a literal, like srand(200) and running the program several times, you will get the exact same results.

If you want different results each time you have to seed with something that will be different each time the program runs, so time is often used as a seed. In your case you are in a very tight loop so many of the calls in a row use the same time value.

If you call srand() once, before your loop this problem will go away.

DaveB
  • 342
  • 1
  • 9
  • I liked your example for time. I think I understand this now. Moving that outside really helped me out. I realize others stated this, but I wasn't sure where to move the srand() in my code. Thank you! –  Apr 21 '15 at 01:30