Reputation: 31

whats wrong with my functions

void getLottoPicks()
{
    int i,k, num[7], num1;

    bool nodup(int num[7],int k, int num1), noover= true;
    for(i=0; i<7; i++)
        num[i]=51;

    cout << "please enter the seven numbers you think will win the lottery: "<< endl;
    for (i=0; i<7;i++)
    {
        cin >> num[i];
        if(num[i] < 1 || num[i] > 40)
        {
            i= i-1;
            noover = false;
            cout << "The number is out of range. Please enter a new number ";
        }

        if(noover)
        {
            num1=num[i];
            k=i;
            if (!nodup(num,k, num1))
            {   
                i=i-1 ;
                cout <<" you type the duplicated number. please enter another number"; 
            }
        }
    }
}
bool nodup(int num[7],int i, int num1)
{
    bool dup = false;
    int k;
    for(k=0; k<i-1; k++)
    {
        if (num[k] = num1)
            dup = true;
    }
    return !dup;
}

every time i enter 2 of the same number it always couts " you type the duplicated number. please enter another number";

Upvotes: 0

Answers (6)

Sarfaraz Nawaz

Reputation: 361442

The problem is obviously with if (num[k] = num1) which should be written as if (num[k]== num1). That is, change the assignment operator to equality operator.

Also the for loop's condition should be this:

for(k=0; k <= i ; k++)  //in nodup function
       //^^^^^^ I think you want this!

And in getLottoPicks the loop should be this:

for (i = 0 ; i<7;i++)
    //^^^^ start from zero

By the way, if I understood your code (and the job it does) correctly, then I think you should use std::set which automatically solves many problems. First and foremost is, std::set always checks for duplicate before adding new item to itself. That means, every item in std::set is unique.

Consider using : std::set<int>

If you use std::set<int>, your code would reduce to this:

#include <set>

void getLottoPicks()
{
    set::set<int> nums;
    int input;
    while(nums.size() < 7)
    {
        std::cin >> input;
        if(input < 1 || num > 40 )
        {
            std::cout << "Out of range. Please enter number between [1-40] : ";
        }
        else if(nums.insert(input).second == false)
        {
            std::cout <<"Duplicate number. Please try again : "; 
        }
    }
}

And you don't need nodup() function anymore.

Upvotes: 1

Aamir

Reputation: 15566

There are quite a few problems in your code technically and aesthetically.

Your for loop for (i=1; i<7;i++) is wrong. This will only run 6 times. You should try starting it with i=0.
Instead of doing a forward declaration like: bool nodup(int num[7],int k, int num1), noover= true;, either move your forward declaration before the method getlottopicks or move the whole method nodup before lotto picks. This somehow doesn't seem easy on the eye.
if (num[k] = num1) is wrong. Replace it with if (num[k] == num1)
use i-- instead of i=i-1 and use continue instead of using a variable noover.

Fix these and your code will be a lot better.

Upvotes: 1

Maurice Perry

Reputation: 32831

There is another issue: the for loops in getLottoPicks() start from 1 while that of nodup() starts from 0.

Upvotes: 1

Dusty Campbell

Reputation: 3156

You are filling your array starting at different positions. In the getLottoPicks() you are starting with 1 and in nodup() you start at 0. In C++ you generally start arrays at 0.

Upvotes: 3