Help! Matching numbers

[DesmondLee]

The user has to guess 6 numbers before they are generated. Your program will then tell the
user what are the matched numbers. If the user chooses any number less than 1 or more
than 42, your program has to prompt the user to key in another set of 6 numbers. Your
program should ask the user whether he/she wants to continue playing the game or not. If
the user chooses to stop, the main menu will be displayed.

Example :
Example 1:
Please choose 6 numbers between 1 until 42:
2 5 8 13 22 31
The 6 random numbers generated are:
22 16 23 2 39 40
Matched numbers(s):
2 22



Problem : My code doesn't print the matched numbers output.

  #include "stdafx.h"
#include <iostream>
#include <ctime>
#include <cstdio>
#include <cstdlib>
#include <iomanip>
#define MAX 42
#define MIN 1
#define SIZE 6
using namespace std;


void display();
void inputs();
//int generate_number();
void generate_number();

//int check_Inum(int);
int rng[SIZE];
int i;
int j;
bool match = true;

int main()
{

	display();
	inputs();

	return 0;
}

void display()
{
	cout << "=============================================================" << endl;
	cout << "||                         NUMBER                          ||" << endl;
	cout << "||                        MATCHING                         ||" << endl;
	cout << "||                          GAME                           ||" << endl;
	cout << "=============================================================" << endl;
	cout << endl;

}

void inputs()
{
	int i, inum[6];
	bool check = true;

	// Input Values
	do
	{
		check = true;
		cout << "Please enter a series of 6 numbers between 1 until 42 : " << endl;

		for (i = 0; i < SIZE; i++)
		{
			cin >> inum[i];
		}

		for (i = 0; i < SIZE; i++)
		{
			if (inum[i] < MIN || inum[i] > MAX)
				check = false;

		}

		if (check == false)
		{
			cout << "The number that you have entered is out of range. \n";
		}

	} while (check == false);
	
	generate_number();
	cout << endl;
	cout << "Matched Number(s) :\n";
	for (i = 0; i < SIZE; i++)
	{
		const int matchnum = rng[i];
		for (j = 0; j < SIZE; j++)
		{
			if (matchnum == inum[j])
			{
				cout << matchnum << " ";
			}
		}
	}
}

void generate_number()
{
	int rng[SIZE];
	int i;
	int range = (MAX - MIN) + 1;
	srand((unsigned int(time(NULL))));//Seed temporarily
	for (i = 0; i < SIZE; i++)
	{
		rng[i] = rand() % range + MIN;
		cout << rng[i] << " ";
	}
}

[jonnin]

inside input function, what is RNG? Its not the one from generate number ... this is the problem.

aside from that, consider:
int tbl[43] = {0};
tbl(rand()%42) = 1;
...
if tbl[userinput] == 1 they guessed it, else not.
don't need all the looping and extra logic, you are doing too much work. you can replace the double for loop entirely. you can also make the validity checks much more simple. Nothing you have is wrong, you are just doing a lot more than needed.

[tpb]

If this is supposed to simulate one of those ball-drawing lotteries then you probably shouldn't allow duplicates, either in the user's input or the generated numbers.

[Handy Andy]

Hello DesmondLee,

Starting at the top of your program:

You include "<iomanip>", but never use it although you could.

The use of the "#define"s is OK, but I would have written them as constexpr size_t MAX{ 42 }; where you can think of "size_t" as another name for an "unsigned int".

It is best to avoid using "using namespace std". This WILL get you in trouble some day. I found this is worth reading http://www.lonecpluspluscoder.com/2012/09/22/i-dont-want-to-see-another-using-namespace-xxx-in-a-header-file-ever-again/ you can also do a search here to find more information.

The prototypes are OK.

The use of global variables should be avoided and it is giving you a problem with the program. More later.

The "main" function is missing some code. When you have a program that uses "rand()" it is best to seed the RNG at the beginning of main. There are several ways to write this code, but I prefer to use this srand(static_cast<size_t>(time(0)));. The "static_cast<size_t>()" is because "srand()" takes an unsigned int. "srand()" only needs to be done once. Usually done at the beginning of "main".

At the end of main just before the return I added this code to keep the console window open to see the output.

I will start with the "generate_number()" function first and the first problem. At the beginning of the file you defined "int rng[SIZE];". Inside the function you defined "int rng[SIZE];" again. this local variable overshadows the global variable and the function used the local variable. When the function ends so does the local variable, so the global variable never receives any numbers. You could either do away with the global variable and make the local variable "static" so it is not lost or do away with the local variable thus using the global variable.

Normally what i do is define the variables in main and pass them to the functions that need them. In your case the array "rng"would have to be passed to two functions. The array name "rng" may be simple to you, but you should use a better name that better describes what the variable is like: "generatedNumbers". It feels like now is the time to learn to use a more descriptive name than a cryptic name.

It is not the best idea to put the "srand()" in the function because it could be called more than once and it only needs to be done once. I have not looked into it, so it may work here, but a bad habit to get into.

The function "inputs()" appears to be OK although I have not properly tested it properly.

In your for loops you start with for (int i =0 yet you not only defined "i" in the function, but as a global variable. You do not need to define a global variable or a local variable for "i" because the for loop defines a local variable for "i" which is lost when the for loop ends and the other local variable "i" and the global variable "i" are never used.

In the end I commented out some of the local variables defined in the functions like "i" and "rng" forcing the program to use the global variables except "i" and the program ran fine.

Hope that helps,

Andy

[tpb]

Adding to Andy:

Don't include <cstdio> unless you really need it (which you generally shouldn't since <iostream> is better).

Use const int instead of define for MAX, MIN, and SIZE. (Or constexpr if you're using C++11 or better, which you should.)

srand should only be called once in the entire run of the program. It's best to call it early in main.

You shouldn't use global variables. The reason your program isn't working is that you've defined the arrays as both global and local. You fill the local array, but in the other functions they access the global array.

So the first part of your program should perhaps look something like this (although this doesn't have the loop that allows the user to play again):

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

using namespace std;   // consider not using that and putting std:: in front of library functions

const int MAX  = 42;    // or constexpr
const int MIN  =  1;
const int SIZE =  6;

void display();
void inputs(int *inum);
void generate_number(int *rng);
void check_matches(int *inum, int *rng);

int main() {
    srand(time(0));  // In C++11 you can use nullptr instead of 0 (but don't use NULL).
    display();

    int inum[SIZE];
    int rng[SIZE];

    // you'll want to loop this part to allow the user to play again
    inputs(inum);
    generate_number(rng);
    check_matches(inum, rng);
}

[jonnin]

one last thing... consider this:

srand(31415); //your favorite *constant* value

for testing purposes. After running it, you will know the values and can enter them and 'guess' correctly (each run will be exactly the same as the last until you put it back to using time).

that way you can successfully guess the values and test your code changes.
just remember to put it back afterwards, or it won't be much of a game :P

[Handy Andy]

Hello DesmondLee,

I worked on your program for a bit and offer this as a possible solution to what you had. If there is anything you do not understand let me know.

#include <iostream>
#include <ctime>
#include <cstdio>
#include <cstdlib>
//#include <iomanip>

//using namespace std;

constexpr int MAX{ 42 };
constexpr int MIN{ 1 };
constexpr int SIZE{ 6 };

void display();
void inputs();
void generate_number(int generatedNumbers[SIZE]);


int main()
{
	//srand(static_cast<size_t>(time(0)));
	srand(static_cast<size_t>(31415));

	display();
	inputs();

	// The next line may not be needid. If you have to press enter to see the prompt it is not needed.
	std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');  // <--- Requires header file <limits>.
	std::cout << "\n\n Press Enter to continue";
	std::cin.get();

	return 0;
}

void display()
{
	std::cout << "=============================================================" << std::endl;
	std::cout << "||                         NUMBER                          ||" << std::endl;
	std::cout << "||                        MATCHING                         ||" << std::endl;
	std::cout << "||                          GAME                           ||" << std::endl;
	std::cout << "=============================================================" << std::endl;
	std::cout << std::endl;

}

void inputs()
{
	int inum[6];
	int generatedNumbers[SIZE];
	bool check = true;

	// Input Values
	do
	{
		check = true;

		std::cout << "Please enter a series of 6 numbers between 1 until 42 : " << std::endl;

		for (int i = 0; i < SIZE; i++)
		{
			std::cout << ' ' << i + 1 << ". ";
			std::cin >> inum[i];

			if (inum[i] < MIN || inum[i] > MAX)
			{
				std::cout << "\n The number that you have entered is out of range. \n";
				i -= 1;
			}
		}
	} while (!check);

	generate_number(generatedNumbers);
	
	std::cout << std::endl;
	std::cout << "\n Matched Number(s) are:\n ";
	
	for (int i = 0; i < SIZE; i++)
	{
		const int matchnum = generatedNumbers[i];

		for (int j = 0; j < SIZE; j++)
		{
			if (matchnum == inum[j])
			{
				std::cout << matchnum << " ";
			}
		}
	}
}

void generate_number(int generatedNumbers[SIZE])
{

	std::cout << "\n Generated numbers are:\n ";

	for (int i = 0; i < SIZE; i++)
	{
		generatedNumbers[i] = rand() % MAX + 1;
		std::cout << generatedNumbers[i] << " ";
	}
}

Andy

[tpb]

@Andy,

Why would you include <cstdio>?

It is an error to cast the argument to srand to a size_t. srand takes an unsigned int, whereas size_t is usually an unsigned long.

And why would you just hand out a (relatively) complete program, anyway? Especially one so badly structured?

[Handy Andy]

@tpb,

because I forgot to take it out.

Andy

[DesmondLee]

Thank you everyone. Finally solved it. I just learn C++ recently. Thanks for all the knowledge.

[icy1]

Check the case of a user entering bad numbers, then wanting to play again. I found that when I broke out of the loop, the input stream got into an error state, so needed to clear/ignore some characters at that point.

#include <iostream>
#include <random>

using namespace std;

int main()
{
    const int TOTAL = 6;
    const int LOW = 1;
    const int HIGH = 42;

    mt19937 rd(1);  // Change this to empty constructor to not control the seed
    bool redo;
    char again;
    int nums[TOTAL];
    int rng[TOTAL];

    do
    {
        cout << "\nPlease enter " << TOTAL <<" space-separated numbers from " << 
                LOW << " to " << HIGH << '\n';
        redo = false;        
        for (int i=0; i<TOTAL; ++i)
        {
            cin >> nums[i];
            if (nums[i]<LOW || nums[i]>HIGH)
            {
                cout << "Sorry, one of your numbers is out of range.  Restarting.\n";
                redo = true;
                again = 'y';
                cin.clear();
                cin.ignore(1000, '\n');
                break;
            }
            rng[i] = rd()%(HIGH-LOW+1) + LOW;
        }
        if (redo)
            continue;

        cout << "Randomly generated nums:\n";
        for (auto r : rng)
            cout << r << ' ';                 

        cout << "\nMatched nums:\n";
        for (auto n : nums)
            for (auto r : rng)
                if (n==r)
                    cout << n << ' ';               

        cout << "\nPlay again (y/n)? ";
        cin >> again;
        cin.clear();
        cin.ignore(1000, '\n');
    }
    while (again == 'y' || again == 'Y');

    return 0;
}

Edit: Also noticed an answer or guess could contain repeated numbers (seed 1, second set will always contain two 36). Iterating through each set could result in duplicates printed out. This could be solved by storing matches in a set, or by keeping track of matched indices after the nested pass, and only print out these indices later. Example output:

Please enter 6 space-separated numbers from 1 to 42
 36 6 36 25 33 28
Randomly generated nums:
8 18 19 39 32 14 
Matched nums:

Play again (y/n)?  y

Please enter 6 space-separated numbers from 1 to 42
 36 6 36 25 33 28
Randomly generated nums:
36 6 36 25 33 28 
Matched nums:
36 36 6 36 36 25 33 28 
Play again (y/n)?  no thx

[jonnin]

you are doing great. Now that you have it working consider these ideas next time:

use <random>. rand() is a low quality tool from C and the c++ replacement is recommended.
use <vector> instead of arrays.
make the input function just do input and test
add a play-game or something function to check for matches
loop in main to play again if desired
get rid of the globals and pass parameters.
change # defines to something else as said already
use simple logic, complexity is the enemy three times over, once in writing it, once in debugging it, and slower execution every time it runs.
------------------------------------------------------------
here is a far from perfect but a good starting place for these ideas.

enum //there are many ways to do this. This is one of them, as the others are mentioned already
//Ideally you would wrap enums in a namespace for large code and name them
{
eMAX = 42, eMIN = 1, eSIZE = 6, eMAX1 = 43    
};

void display();
void inputs(vector<int> &inum); //changed to only handle user input. 
void generate_number(vector<int> &rng);
void play_game(vector<int> &inum, vector<int> &rng); //moved the rest of inputs here


int main()
{
    vector<int> rng(eMAX1);
    vector<int> inum(eSIZE);
    int keep_playing = 0;
	display();
    do
    {
	inputs(inum);
    generate_number(rng);
    play_game(inum,rng);
    cout << "play again? 0 to quit\n";
    cin >> keep_playing;
    } while (keep_playing);
    
	return 0;
}

void display()
{
	cout << "=============================================================" << endl;
	cout << "||                         NUMBER                          ||" << endl;
	cout << "||                        MATCHING                         ||" << endl;
	cout << "||                          GAME                           ||" << endl;
	cout << "=============================================================" << endl;
	cout << endl;
}

void inputs(vector<int> &inum)
{ //radical redesign checks each number and only re-input the bad one each time.
//also simplifies logic. 
    bool check = false;      
    cout << "Please enter a series of 6 numbers between 1 and 42 : " << endl;
		for (int i = 0; i < eSIZE; i++)
		{			
            do
            {
                cin >> inum[i];
                check = true;
                    if (inum[i] < eMIN || inum[i] > eMAX)
                        {
                            check = false;
                            cout << "The number that you have entered is out of range. \n";
                        }
            } while (check == false);    
		}				
}

void play_game(vector<int> &inum, vector<int> &rng) //moved the rest of inputs here
{
    cout << "The random numbers were\n";
    for (int i = 0; i < eMAX1; i++)
        if(rng[i]) cout << i << " ";      
	cout << "\n\nMatched Number(s) :\n";
	for (int i = 0; i < eSIZE; i++)
	{
		if(rng[inum[i]])
		cout << inum[i] << " ";			
	}
    cout << endl;
 }


void generate_number(vector<int> &rng)
{
    //redesigned for c++ instead of C and to use bucket approach
    //set up a c++ random generator
static bool once = true;    
static std::default_random_engine generator;
    if(once)
    {
        generator.seed(std::random_device()());
        once = false;
    }
static std::uniform_int_distribution<int> distribution(1,42);

  //clear the table for a new group 
    for (int i = 0; i < eMAX1; i++)
        rng[i] = 0;
//get a new random group
    for (int i = 0; i < eSIZE; i++)
        {
            rng[distribution(generator)] = 1;  		
        }		
}

Again, mine can also be better, I didn't have much time to rearrange it. The web thing messed with my indents also, sorry about that.