-1

As a part of code for a certain game I want to generate 4 unique random numbers into a vector.

This code works for some number of repeated plays and then application crashes (not responding window).

While I understand that if-condition prevents for-loop from inserting the same number into a vector, how much time does this for-loop takes until it generates unique numbers via rand() function? How srand(time(NULL)) and rand() exactly work together to create random values depending on the system time?

#include <iostream>
#include <vector>
#include <algorithm>
#include <cstdlib>
#include <ctime>

using namespace std;

//plays bulls and cows


int main() {
srand(time(NULL));
string play="yes";
int nums=4;       // number of values in an answer (must NOT exceed 10)
vector<int> answer;


while (play=="yes" || play=="YES" || play=="Y" || play=="Yes" || play=="y") { //plays the game

answer.push_back(rand()%10+1);
  do {                              //fills vector with unique random numbers
    for (int i=1; i<nums; i++) {
      answer.push_back(rand()%10+1);
      if (answer[i]==answer[i-1]) {
        i=i-1;
        continue;
        }
      }
  } while (answer.size()!=nums);

for (int i=0; i<nums; i++) {
  cout<<answer[i];
}

  cout<<"Do you want to play again?"<<'\n';
  cin>>play;
  answer.clear();
} //game ends


if (play=="no" || play=="n" || play=="No" || play=="NO" || play=="N") { //terminates and checks for exceptions
  cout<<"Thank you for playing!"<<'\n';
  return 0;
} else {
  cerr<<"Error: wrong input. Terminating."<<'\n';
  return 0;
}

    return 0; //safety return
}
bruneleski
  • 33
  • 1
  • 5
  • Also, I know there is better, C++11 way to do this, but I need to do it this way. – bruneleski Jan 10 '17 at 04:03
  • Consider using a `std::set` instead of `std::vector`. A set only stores unique values: `std::set answer; while (answer.size() < 5) answer.insert(your_random_number);` – PaulMcKenzie Jan 10 '17 at 04:06
  • That's cool. But we work with vectors now, so not sure if they'll accept set. (exercise kind of a thing) – bruneleski Jan 10 '17 at 04:12
  • 1
    Then use a set and copy the results to your vector. No need to write error-prone loops. – PaulMcKenzie Jan 10 '17 at 04:16
  • @PaulMcKenzie, that would be one way of doing it. Thanks. – bruneleski Jan 10 '17 at 04:21
  • There are so many dups it's scary. Whenever someone tries to devise a scheme to ensure a sequence of random numbers is unique, they invariably break the distribution properties of the PRNG. Please make sure you understand the consequences of what you're doing. – Disillusioned Jan 10 '17 at 04:21
  • If you are to use a set, you may need to shuffle the vector after the items are copied to the vector, since set also stores items in order. If you must not use C++11, then there is `std::random_shuffle` which will shuffle the array. Otherwise, `std::shuffle`. [See this](http://en.cppreference.com/w/cpp/algorithm/random_shuffle) – PaulMcKenzie Jan 10 '17 at 04:28

3 Answers3

0

Why do you add the new try into answer instead of temporary variable. If the variable is valid, then add it into the answer. In your case, i always keep at 1;

while (play=="yes" || play=="YES" || play=="Y" || play=="Yes" || play=="y") { //plays the game

    int last_try=rand()%10+1;
    answer.push_back(last_try);
    do { //fills vector with unique random numbers
            int new_try=rand()%10+1;

            if (last_try!=new_try)
            {
                answer.push_back(new_try);
                last_try=new_try;
            }
    } while (answer.size()!=nums);


    for (int i=0; i<nums; i++)
    {
        cout<<answer[i]<<"\n";
    }

    cout<<"Do you want to play again?"<<'\n';
    cin>>play;
    answer.clear();
} //game ends
jeremine
  • 65
  • 1
  • 7
  • That's more reasonable than my stuff. Thanks. But doesn't the core idea remains the same? I mean it occurs to me that it somehow bruteforces for-loop, because if I tried doing it without do-while loop, srand() with rand() always return same integers. – bruneleski Jan 10 '17 at 04:19
  • @bruneleski, It is true. The idea is to brute-force. Your previous code is highly vulnerable. You have a for loop `for (int i=1; i – jeremine Jan 10 '17 at 04:25
  • Well I thought I took care of that with answer.clear() – bruneleski Jan 10 '17 at 04:29
  • If you replaced `while (answer.size()!=nums);` with `while (answer.size() – jeremine Jan 10 '17 at 04:30
  • Oh I get it now! Your answer in combination with @MyUsername112358 's clicked it for me. Thanks a ton. – bruneleski Jan 10 '17 at 04:39
0

Assume that you must use std::vector (and not a std::set). The easiest way to fill your vector with random numbers is to check to see if the number has already been "seen" -- if not, then add it to the vector.

This can be accomplished by using an array of bool as a helper to determine if the number has been seen:

#include <vector>
#include <iostream>
#include <cstdlib>

int main()
{
    std::vector<int> answer;
    int num = 4;

    // 10 numbers
    bool seen[10] = {false};

    // keeps track of numbers added
    int numsAdded = 0;
    while (numsAdded < num)
    {
       int numRand = rand()%10;
       if ( !seen[numRand] )
       {
         // not seen, so add it to vector and update bool array and
         // numsAdded
         answer.push_back(numRand + 1);
         seen[num] = true;
         ++numsAdded;
       }
    }
    for (size_t i = 0; i < num; ++i)
       std::cout << answer[i] << " ";
}

Live Example

PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45
-1

The problem is you always push back the random value in your vector before checking if it's valid. Let's say your program generates these random value in order:

2, 6, 6, 7, 9, 10

What happens is you will insert 2 (i == 2), 6 (i == 3), 6 (i == 4), then realize 6 is repeated twice, so you go back one iteration (i == 3), but both your sixes are still in your vector. So now you will add 7 (i == 4) and you will exit the for loop with 5 values in your vector.

Then when you evaluate your do-while condition, your answer.size() won't ever equal 4 because it already is equal to 5. You are now stuck in an infinite loop, and your application crashes when it consumes all the available memory from your vector growing infinitely.

Also, you appear to have an error in your logic. To make sure you don't have a repeated value (and you are stuck with vectors), you should not only validate the last inserted value but the whole vector. Like this:

#include <algorithm>

if ( std::find(vector.begin(), vector.end(), item) != vector.end() )
   do_this();
else
   do that();
MyUsername112358
  • 1,320
  • 14
  • 39