0

I am writing a code that is supposed to act as a lottery. The lottery numbers available are 1-50, and there are 10 of them. The user is supposed to input one number and the program returns if the user's number matches one of the 10 random lottery numbers. I have gotten all of these parts down, but there is one problem; all 10 of the lottery numbers must be unique. I have gotten 10 unique numbers 1-50, but they weren't very random. The code I have written up to this point seems correct to me, except I know there is something missing (along with the fact that I can clean my code up a lot, but I'm focusing on the objective right now). Right now if I run the program it will return ten zeroes. I need each element in the lottery array to be a unique number from 1-50, and produce different sets of numbers each time i run the program. Any help would be appreciated.

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

using std::cout; using std::cin; using std::endl;

void printOut(int[]);
void draw(int[]);
bool check();
int fillFunc[10];

int main()
{
    const int arraySize = 10;
    int win[arraySize] = {};

    srand((unsigned)time(NULL));

    draw(win);
    cout << "Your lottery numbers are: ";
    printOut(win);
}

void draw(int fillFunc[])
{
    int i;
    for (i = 0; i < 10; i++)
    {
        if (check() == true)
            continue;
        fillFunc[i] = 1 + rand() % 50;
    }
}

void printOut(int fillFunc[])
{
    for (int i = 0; i < 10; i++)
    {
        cout << " " << fillFunc[i];
    }
    cout << "\n";
}

bool check()
{
    for (int i = 0; i < 10; ++i)
    {
        if (fillFunc[i] == i)
            return true;
    }
    return false;
}

(also don't ask me why the array has the name "win", this is what my prof wants me to call it)

Retired Ninja
  • 4,785
  • 3
  • 25
  • 35
Breinz
  • 31
  • 4
  • This is why braces are important. Good indentation would also give you a clue something wasn't right. – Retired Ninja Oct 11 '17 at 04:11
  • You should watch this: [rand() Considered Harmful](https://channel9.msdn.com/Events/GoingNative/2013/rand-Considered-Harmful) – Jesper Juhl Oct 11 '17 at 04:14
  • 1
    What is `check()` supposed to be checking? – M.M Oct 11 '17 at 04:18
  • I assume it's supposed to be checking if the number has already been chosen to keep them unique, but that's not what it does at all. – Retired Ninja Oct 11 '17 at 04:19
  • I would like to add that my indentation is not as pictured in this post. I pasted it and by hand indented every line 4 spaces, but not the lines that were already past 4 spaces so that it would show up as code and not plain text. – Breinz Oct 11 '17 at 04:21
  • Check is used for two things. One, for checking if the number the user inputs matches any of the lottery numbers, which i haven't implemented yet at all. And two, for checking to see that the numbers are unique in the lottery array. I understand it does nothing, and now that i have fixed the random component of the array, it is the other problem i am seeking help with – Breinz Oct 11 '17 at 04:22
  • Two easy ways to generate 10 unique numbers in the range 1-50: 1) put all 50 numbers in a `std::vector` or `std::array`, `std::shuffle` it and grab the first 10. 2) Put a randomly generated number in the range into a `std::set` and just keep doing that until the size of the set is 50. – Jesper Juhl Oct 11 '17 at 04:23
  • Please don't make corrections to the code in the question which make an existing answer obsolete. – aschepler Oct 11 '17 at 04:42
  • Alright i will revert the code back to its original state. – Breinz Oct 11 '17 at 04:44
  • 1
    I formatted the code for you, but the easy way to do it in the future is paste the formatted code in the question, select all of the code and use the `{}` button to indent it all 4 spaces so it's recognized as a code block. – Retired Ninja Oct 11 '17 at 05:00
  • Didn't realize i could do that. New to posting here as you can tell. Thanks for the response. – Breinz Oct 11 '17 at 05:04

4 Answers4

1

To answer your question that you updated in the comments you need to consider a shuffling algorithm.

I will give you an idea of how to do this that is O(n) so you don't have to "loop" thru your current list of numbers and keep checking to see if the new number was already picked...

your lottery max number is 50 so make an array of size 50 that is as follows:

  • lotteryNumber[0]=1
  • lotteryNumber[1]=2
  • ...
  • lotteryNumber[49]=50

to pick a number to put into your "selected" numbers array...

indexToLotteryNumbers = rand() % 50 - numbersPickedSoFar;
randomLotteryNumber[i++] = lotteryNumber[ indexToLotteryNumbers ];

// this is the key "trick"
swap(&lotteryNumber[ indexToLotteryNumbers ], &lotteryNumber[49-numbersPickedSoFar]); 

numbersPickedSoFar++;

Theory

  1. The random numbers are the indexes and not the actual values
  2. By swapping the number you picked with the outer element you don't care if your random number generator picks the same index because it will be different the next time.

look at a small example...say you have 1 2 3 4

  • rand() generates "2"
  • so your first # is 2 (assume 1-based indexing here) now you swap the (4th) element for the (2nd) element now you have 1 4 3 |2
  • (here |2 means you can't pick that number again because it's outside the random # generator range) but you reduce the random # generation from 1-4 to 1 thru 3
  • rand() generates "1" so your number is "1" and you swap that with the "outer" unpicked number 3 4 |1 2
  • rand() generates
  • "2" again! your lottery number is 4 this time... and so on.

I hope this makes sense.

alexander.polomodov
  • 5,396
  • 14
  • 39
  • 46
1

Because check() always return true. At the start, the array is filled with zeroes so check returns true as win[0]==0 and that remains true forever as nothing changes that value. So in draw you always branch to continue and never modify anything.

To solve the problem one way could be to shuffle the sequence of 1-50 and extract the first 10 values of the shuffled array. You can use the very simple Fisher-Yates algorithm.

Jean-Baptiste Yunès
  • 34,548
  • 4
  • 48
  • 69
0

You only assign a value to fillFunc[i] when i is 10, which is an out of bounds access. Move the assignment to inside the loop.

You have other problems too. If you do continue, you just leave an entry in the array unset.

You should try adding a lot of output statements to your program so you can more easily understand what it's doing. If you prefer, use a debugger to step through it.

David Schwartz
  • 179,497
  • 17
  • 214
  • 278
  • The entries were initialized to `0` by `int win[arraySize] = {};`; they're not unset – M.M Oct 11 '17 at 04:17
  • I have adjusted my for loop and it now produces random numbers again which is good, but as you said the continue does nothing. I guess my question is how can i use my check function to detect when the number was already used in the array, and produce a different number in its place? – Breinz Oct 11 '17 at 04:19
  • Oh, I see that now. Updated my answer. – David Schwartz Oct 11 '17 at 04:30
  • @Breinz An ugly solution would be to decrement `i` before the `continue`. – David Schwartz Oct 11 '17 at 04:31
  • an ugly solution is what i need because my prof does not want me using anything like vectors or other algorithms such as random shuffle. I used the decrement in the if statement but it just runs the program forever without selecting a single number. What other way would you write it? This is how i wrote it `if (check() == true){ i--; continue; }` – Breinz Oct 11 '17 at 04:47
  • Set `fillFunc[i]` back to zero before decrementing `i`. Also, make your check function do something sane. – David Schwartz Oct 11 '17 at 04:51
0

This is what I would do, given your constraints. Rather than check if a number is unique while you are filling the array, just pass the array into the function that chooses the number so it can return a unique value.

I also removed the redundant global array. It could be a source of bugs if you forgot to pass the local array you were working with to any of the functions.

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

using std::cin;
using std::cout;
using std::endl;

int getUniqueNumber(int fillFunc[])
{
    while(true)
    {
        //pick number
        int val = 1 + rand() % 50;
        //assume it's unique
        bool unique = true;
        for (int i = 0; i < 10; ++i)
        {
            //if another number matches, it isn't unique, choose again
            if (fillFunc[i] == val)
            {
                unique = false;
                break;
            }
        }
        //if it is unique, return it.
        if (unique)
        {
            return val;
        }
    }
    //never reached, but avoids an all control paths must return a value warning.
    return -1;
}

void draw(int fillFunc[])
{
    for (int i = 0; i < 10; i++)
    {
        fillFunc[i] = getUniqueNumber(fillFunc);
    }
}

void printOut(int fillFunc[])
{
    for (int i = 0; i < 10; i++)
    {
        cout << " " << fillFunc[i];
    }
    cout << "\n";
}

int main()
{
    srand((unsigned)time(NULL));
    const int arraySize = 10;
    int win[arraySize] = {};
    draw(win);
    cout << "Your lottery numbers are: ";
    printOut(win);
    return 0;
}

There are other, perhaps better, ways to select unique numbers in a range, but I went for simple to implement and explain. You can read about some other methods in these questions:

Unique random numbers in an integer array in the C programming language

Unique (non-repeating) random numbers in O(1)?

Retired Ninja
  • 4,785
  • 3
  • 25
  • 35