Write a program that simulates a lottery

I am taking a C++ course and we have received our final assignment, I was looking for some suggestions and or tips so I can continue to improve.

I am aware that the use of using namespace std is bad practice but my professor prefers that we use this way since it is a beginners course.

The assignment was a two part assignment I have commented out the piece of the code that was unneeded for the second part.

Part One:


Write a program that simulates a lottery. The program should have an array of 7 integers
named winningDigits, with a randomly generated number in the range of 0 through 9 for each element in the array. The program should ask the user to enter 7 digits and should store them in a second integer array named player. The program must compare the corresponding elements in the two arrays and count how many digits match.


For example, the following shows the winningDigits array and the player array with sample numbers stored in each. There are two matching digits, elements 2 and 4.


WinningDigits: 7 4 9 1 3 5 0


Player: 4 2 9 7 3 4 2

Part Two:


Modify the lab solution so that the player numbers are entered not from the keyboard, but from the file player.dat (you will need to create this file and add 7 numbers).

What can I do better?

```
#include
#include
using namespace std;

int main()
{
const int size = 7;
int winningDigits[size];
int player[size];
int count=0;
ifstream datafile;

datafile.open("player.dat");

for(int i=0; i> player[i];
if (winningDigits[i]==player[i])
count++;
}
/*
for(int i=0; i> player[i];

if (winningDigits[i]==player[i])
count++;
}
*/
cout << "WinningDigits: ";
for(int i=0; i<size; i++)
cout << winningDigits[i] << " ";

cout << "\nPlayer: ";
for(int i=0; i<size; i++)
cout << player[i] << " ";

cout << " \nThere are " << count << " matching elements ";
for(int i=0;

There's quite a few things I would change here. Getting quality random number generation right is difficult, (these questions come up a lot so it's something I've blogged about), but thankfully with the newer c++11 standard things are a fair bit easier.
Program Correctness

Making sure that your program is correct is extremely important. As it stands this program is not correct (as in does not meet the specification):

cout << " \nThere are " << count << " matching elements ";
for(int i=0; i<size; i++){
    if(winningDigits[i]==player[i]){
        cout << i+1 << " ";
    }
}
cout << "\n";

This comparison assumes that the arrays are sorted, which is not the case. So matching elements could be completely missed here. Sort the arrays first before comparing them or consider a different data structure to store the results from generating the numbers.
Basic things:

There's commented out code in the source, this is almost always a bad sign. Keep code changes tracked by version control, not commented out blocks.

Loops like this are a nightmare formatting wise:

for(int i=0; i<size; i++)
cout << winningDigits[i] << " ";

I personally always use braces (as does the coding standard we go by at my work) but whatever you do you need a tab here:

for(int i=0; i<size; i++){
    cout << winningDigits[i] << " ";
}

Another thing is this:

using namespace std;

Other people have commented on this, but generally speaking the only time you would really want to do this is when you are making some small throwaway program for testing a concept or making an example or similar. You probably don't want to do it in any production code. This pollutes the namespace which is something you don't want, see this https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice for more info on that.

If you want to cut down on on typing you can bring in just the names you need by doing:

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

an so on.
Standard library containers

Generally speaking with c++ you don't want to use raw arrays, the standard container classes are safer and allow you to be more productive. There's a c++ FAQ article that deals with just this, so give that a read.

int winningDigits[size];
int player[size];

Instead of these I would use std::array for both of these as the length is both fixed and known at compile time. Were then lengths not known then std::vector would most likely be the best choice. These become:

#include  //header needed for std::array

std::array winningDigits;
std::array player;

Now this ends up being the same type in a bunch of different places so a typedef will help make things easier:

typedef std::array lottery_results_t;
lottery_results_t winningDigits;
lottery_results_t player;

An additional benefit of the typedef is that if you do change the type later it will be easier to do so.
Random number generation

rand() is a really bad random number generator, to the point where the standard committee actually made a report about depreciating it. Additionally rand() gets worse when you take the modulus of it (statistically speaking).

We can avoid all these problems by just using the newer c++ random number library in a manner like this:

std::default_random_engine generator;
generator.seed( time(0) );
std::uniform_int_distribution distribution(0,9);//note the min and max parameters are inclusive here
while(true)
{
    std::cout << distribution(generator) << std::endl;
}

Demonstration here: http://ideone.com/xzgcZy

The output code definitely violates the "don't repeat yourself" principle. Success200 has posted a great answer that outlines some of the things you can do to reduce that duplication.

StackExchange Code Review Q#71697, answer score: 7