Keno lottery game

Question

This code implements the basics of the keno lottery game which are:

• There are a total of 60 numbers from 1 to 60 to be picked by the player
• The player can pick 5 to 10 numbers in a game round
• The computer picks 15 random numbers
• Depending on how many numbers are common between the player and the computer chosen numbers, an award is given to the player
• There is also a quick pick functionality, which enables the computer to pick numbers on behalf of the player if he wants
• Maintain credits balance
• 0 balance no game play allowed
• Provide insert credit functionality
• Take a bet input from the player for every game round

Please review the code and let me know the areas of improvement. I have just started learning C++ and this is my first standalone program.

#include <iostream>
#include <vector>

#define MIN_PICK_VALUE 1
#define MAX_PICK_VALUE 60
#define COMPUTER_PICK_COUNT   15

using namespace std;

class RandomNumber{                                         // Random number generator
public:
static int randomNumberGenerator(int min, int max){
return (rand() % max + min);
   }
};

class KenoWinCalculator{

private:
vector<int> m_playerPick;                               // vector is used because the size is not fixed,
int m_ComputerPick[COMPUTER_PICK_COUNT] ={0};           // player can choose any picks between 5 and 10 but computer's is fixed
int m_hits = 0;

public:
KenoWinCalculator(const vector<int> &playerPick){
    for(int i = 0 ; i < playerPick.size() ; ++i)       // Update player picked values
        m_playerPick.push_back(playerPick[i]);
}
void generateComputerPick(){                           // Generate 10 unique computer picked random values

    int rngValue =0, i =0;

    while(i < COMPUTER_PICK_COUNT){
    nextRng:
        rngValue = RandomNumber::randomNumberGenerator(MIN_PICK_VALUE, MAX_PICK_VALUE);
        for(int j =0; j < i; ++j)
        {
           if(m_ComputerPick[j] == rngValue)           // check if the generated rng value is not unique
               goto nextRng;
        }
        m_ComputerPick[i] = rngValue;
        ++i;
    }
}
void calculateHits(){                                  // Calculates no of common nos between player and computer pick
    for(int playerPickCount = 0; playerPickCount < m_playerPick.size();
        ++playerPickCount)
        for(int compPickCount = 0; compPickCount
            < sizeof(m_ComputerPick)/sizeof(m_ComputerPick[0]); ++compPickCount){
            if(m_playerPick[playerPickCount] == m_ComputerPick[compPickCount])
                ++m_hits;
        }
}
int getHitValue() const{
    return m_hits;
}
int getPlayerPickCount() const{
    return static_cast<int>(m_playerPick.size());
}
const vector<int>& getPlayerPickValues(){
    return m_playerPick;
}
int* getComputerPick(){
    return m_ComputerPick;
}
};

class Bank{
private:
int m_bet;
int m_bankBalance;
public:
Bank(): m_bet(0), m_bankBalance(0){}                               // this class maintains bet and player credit meter
void setBet(int betValue){
    m_bet = betValue;
}
void setBankBalance(int balance){
    m_bankBalance += balance;
}
void updatebankBalance(){
    m_bankBalance -= m_bet;
}
int getBankBalance() const{
    return m_bankBalance;
}
bool isEnoughBalanceToPlayGame(){
    return m_bankBalance >= m_bet;
}
bool isBetValid(){
    return m_bet <= m_bankBalance && m_bet > 0;
}
friend class Award;
};


class Award{
private:                                                                       // This class maintains pay table
int m_picks;
int m_hits;

enum Hit{
    HIT_3 = 3,
    HIT_4,
    HIT_5,
    HIT_6,
    HIT_7,
    HIT_8,
    HIT_9,
    HIT_10
};

enum Player_pick{
    PICK_5 = 5,
    PICK_6,
    PICK_7,
    PICK_8,
    PICK_9,
    PICK_10
};

public:
Award(const KenoWinCalculator &calc){
    m_picks = calc.getPlayerPickCount();
    m_hits = calc.getHitValue();
}
void awardWon(Bank &bank){
    int amtWon = 0;
    bool gameLost  = false;
    switch(m_picks){
        case PICK_5: if(m_hits == HIT_5)
                        amtWon = 100;
                    else if(m_hits == HIT_4)
                        amtWon = 50;
                    else
                        gameLost = true;
                    break;
        case PICK_6: if(m_hits == HIT_6)
                        amtWon = 150;
                    else if(m_hits == HIT_5)
                        amtWon = 120;
                    else if(m_hits == HIT_4)
                        amtWon = 100;
                    else
                        gameLost = true;
                    break;
        case PICK_7: if(m_hits == HIT_7)
                        amtWon = 150;
                    else if(m_hits == HIT_6)
                        amtWon = 120;
                    else if(m_hits == HIT_5)
                        amtWon = 100;
                    else if(m_hits == HIT_4)
                        amtWon = 90;
                    else if(m_hits == HIT_3)
                       amtWon = 80;
                    else
                        gameLost = true;
                    break;
        case PICK_8:if(m_hits == HIT_8)
                        amtWon = 175;
                    else if(m_hits == HIT_7)
                        amtWon = 150;
                    else if(m_hits == HIT_6)
                       amtWon = 120;
                    else if(m_hits == HIT_5)
                        amtWon = 100;
                    else if(m_hits == HIT_4)
                       amtWon = 90;
                    else if(m_hits == HIT_3)
                        amtWon = 80;
                    else
                        gameLost = true;
                    break;
        case PICK_9:if(m_hits == HIT_9)
                        amtWon = 200;
                    else if(m_hits == HIT_8)
                        amtWon = 175;
                    else if(m_hits == HIT_7)
                        amtWon = 150;
                    else if(m_hits == HIT_6)
                        amtWon = 120;
                    else if(m_hits == HIT_5)
                        amtWon = 100;
                    else if(m_hits == HIT_4)
                        amtWon = 90;
                    else if(m_hits == HIT_3)
                        amtWon = 80;
                    else
                        gameLost = true;
                    break;
        case PICK_10:if(m_hits == HIT_10)
                        amtWon = 250;
                    else if(m_hits == HIT_9)
                        amtWon = 200;
                    else if(m_hits == HIT_8)
                        amtWon = 175;
                    else if(m_hits == HIT_7)
                        amtWon = 150;
                    else if(m_hits == HIT_6)
                        amtWon = 120;
                    else if(m_hits == HIT_5)
                        amtWon = 100;
                    else if(m_hits == HIT_4)
                        amtWon = 90;
                    else if(m_hits == HIT_3)
                        amtWon = 80;
                    else
                        gameLost = true;
                    break;
    }
    if(!gameLost){
        cout << "You won $" << amtWon << endl;
        bank.m_bankBalance += amtWon;
    }
    else
        cout << "You lost " << endl;
   }
};


void kenoGameplay(Bank &credits){

#define QUIT_VALUE  100
#define MAX_NOS_PICKED  10
#define MIN_NOS_PICKED  5

vector<int> nosPicked;
int num = 0, numCount = 0;
bool isPlayerPickExisting;

char quickPick;
cout << "Do you want computer to pick nos for you? [y]es or [n]o" << endl;
cin >> quickPick;
cin.ignore(numeric_limits<streamsize>::max(),'\n');

if(quickPick != 'y'){
    cout << "Pick atleast 5 and max 10 nos between 1 and 60...Enter 100  to QUIT entering nos\n";
    while(num != QUIT_VALUE && numCount < MAX_NOS_PICKED){
    cin >> num;
    cin.ignore(numeric_limits<streamsize>::max(),'\n');
    if(num == QUIT_VALUE && numCount < MIN_NOS_PICKED){                         // Play game without picking enough nos
        cout << "Game cannot be played. Please pick more nos. \n";
        num = 0;                                                                // Reset num to avoid quiting game
    }
    else if(num == QUIT_VALUE && numCount >= MIN_NOS_PICKED)                    // Play game with enough nos picked
        break;
    else{
        if(num >= MIN_PICK_VALUE && num <= MAX_PICK_VALUE){
           isPlayerPickExisting = false;
            for(int i = 0; i < nosPicked.size(); ++i)
            {
                if(num == nosPicked[i]){
                    cout << "Number picked already exist. Please select another no. \n";
                    isPlayerPickExisting = true;
                }
            }
            if(!isPlayerPickExisting){
                nosPicked.push_back(num);
                ++numCount;
            }
        }
        else
            cout << "You entered invalid no. Please try again \n";
        }
    } // end of while loop
}
else{
        int quickPickCount = 0;                                                         // this code implements quick pick functionality
        cout << "How many nos you want computer to pick (5-10)?" << endl;
        cin >> quickPickCount;
        cin.ignore(numeric_limits<streamsize>::max(),'\n');
        while(quickPickCount < MIN_NOS_PICKED || quickPickCount >  MAX_NOS_PICKED){
            cout << "Invalid count value. Choose between 5 and 10 only. " << endl;
            cin >> quickPickCount;
            cin.ignore(numeric_limits<streamsize>::max(),'\n');
        }

        int rngValue =0, i =0;

        while(i < quickPickCount){
    nextRng:
        rngValue = RandomNumber::randomNumberGenerator(MIN_PICK_VALUE, MAX_PICK_VALUE);
        for(int j =0; j < i; ++j)
        {
            if(nosPicked[j] == rngValue)                                                // check if the generated rng value is not unique
                goto nextRng;
        }
        nosPicked.push_back(rngValue);
        ++i;
        }
    }
KenoWinCalculator winCalc(nosPicked);
winCalc.generateComputerPick();
winCalc.calculateHits();

Award wonAward(winCalc);
wonAward.awardWon(credits);

vector<int> playerPickedValues;
playerPickedValues = winCalc.getPlayerPickValues();

cout << "Your selection: \t";
for(int i = 0; i < playerPickedValues.size(); ++i)                      // display player selected values
    cout << playerPickedValues[i] << "  ";
cout << "\n";

int *computerPickedValues = winCalc.getComputerPick();                  // display computer selected values
cout << "Computer selection: \t";
for(int i = 0; i < COMPUTER_PICK_COUNT; ++i)
    cout << computerPickedValues[i] << "  ";
cout << "\n";
cout << "no of hits: " << winCalc.getHitValue() << endl;                // display no of hits and bank balance
cout << "Your bank balance is: $" << credits.getBankBalance() << endl;
}


int main() {
#define INSERT_MONEY    1
srand(static_cast<unsigned int>(time(0)));
cout << "******** WELCOME TO WORLD OF KENO ************** \n \n" << endl;
cout  << "Please enter your name \n";
std::string name;
cin >> name;
cout << "Hello! " << name << "...\n\n";

char continueGameplay = 'y';
Bank credits;
int insertMoneyReduceBet = 0, bet = 0;

int insertMoney =0;
cout << "Please insert money " << endl;
cin >> insertMoney;
cin.ignore(numeric_limits<streamsize>::max(),'\n');                     // flush cin buffer

credits.setBankBalance(insertMoney);
bool isBetInvalid = true;

while(continueGameplay != 'n'){
    if(credits.isEnoughBalanceToPlayGame() || isBetInvalid == true){     // check if enough balance to play game

        cout << "Place bet " << endl;                                // need bet input from player in case of bet reduction
        cin >> bet;
        cin.ignore(numeric_limits<streamsize>::max(),'\n');          // flush cin buffer
        credits.setBet(bet);

        isBetInvalid = false;
        if(credits.isBetValid()){
            credits.updatebankBalance();
            kenoGameplay(credits);
            cout << "GAME OVER \n";
            cout << "Do you want to play another round \n";
            cout << "'y' to continue and 'n' to quit playing \n";
            cin >> continueGameplay;
        }
        else
            cout << "Invalid bet value." << endl;
    }
    else{
        cout << "Not enough balance/valid bet. Please insert money( press 1) or place valid bet(press 2) to max bank balance "<< credits.getBankBalance() << endl;
        cin >> insertMoneyReduceBet;

        if(insertMoneyReduceBet == INSERT_MONEY){
            cout << "Insert money " << endl;
            cin >> insertMoneyReduceBet;
            credits.setBankBalance(insertMoneyReduceBet);
        }
        else
            isBetInvalid = true;
    }
}
return 0;
}

Answer 1

First, you asked a well defined first question that explains everything in the code so good job!

Include All Necessary Headers

The code is missing some header files, specifically

#include <string>
#include <ctime>

I needed to add these two headers to make the code compile on my computer, which is running Windows 10 Pro and Visual Studio 2015. I would have needed the same 2 header files in either XCode on MacOS or Eclipse on Centos 7.

Avoid the Use of Macros in C++

Macros are a part of C++ to remain backwards compatible with ANSI/ISO C, in new C++ code they should be avoided because they are not type safe. Instead of using #define to define symbolic constants use either const or constexpr.

#define MIN_PICK_VALUE 1
#define MAX_PICK_VALUE 60
#define COMPUTER_PICK_COUNT   15

should be

const int MIN_PICK_VALUE = 1;
const int MAX_PICK_VALUE = 60;
const int COMPUTER_PICK_COUNT = 15;

Prefer Clear Naming over using namespace std

According to the MSDN website:

Namespaces are used to organize code into logical groups and to prevent name collisions that can occur especially when your code base includes multiple libraries.

A collision is when 2 different functions have the same name, the same argument types and a similar functionality (this is why they have the same name). Someone developing software may want to override a function such as std::cout, std::cin or they may want to override the functionality of a class such as std::vector or std::stack. Namespaces allow these constructs to be overridden.

The use of the programming statement:

using namespace std;

hides the fact that cin, cout, vector and stack are coming from the namespace std where cin, cout, vector and stack are used in the code. This can cause confusion of where the code is actually coming from.

As the software becomes more complex and uses more libraries this becomes a bigger problem.

For a more detailed discussion of why it is a bad idea to use using namespace std; see this stackoverflow question and stackoverflow question.

Code Reuse

One of the major goals of object oriented programming is code reuse. Once something is written and debugged it should be available for programming in other projects and programs. It might be better if each of the classes had its own header file (header.h or header.hpp) and a source file. A second goal is encapsulation. Code that uses other classes only needs to know the interfaces and not the details of the implementation. This allows implementations to be imporoved over time.

RandomNumber.h KenoWinCalculator.h and KenoWinCalculator.cpp Bank.h Award.h and Award.cpp kenoGamePlay.h and kenoGamePlay.cpp

The combined use of headers and C++ source files allows the classes/API to be maintained without forcing recompilation of all files. A linking phase may be required.

In some cases these might become dynamic libraries with the header files providing the APIs.

Avoid Using Goto

C++ and to a lesser extent C provide a lot of ways to control the flow of the code, the use of goto is discouraged in most modern programming lanaguages because it is very hard to maintain the code. In the case where it is used here, rather than using a C style integer array it might be better to use a C++/stl container class such as std::array and to use the std::find() function to find any duplicates.

void generateComputerPick() {                           // Generate 10 unique computer picked random values

    int rngValue = 0, i = 0;

    while (i < COMPUTER_PICK_COUNT) {
    nextRng:
        rngValue = RandomNumber::randomNumberGenerator(MIN_PICK_VALUE, MAX_PICK_VALUE);
        for (int j = 0; j < i; ++j)
        {
            if (m_ComputerPick[j] == rngValue)           // check if the generated rng value is not unique
                goto nextRng;
        }
        m_ComputerPick[i] = rngValue;
        ++i;
    }
}

might be rewriten as

#include <array>
#include <algorithm>

void generateComputerPick() {                           // Generate 10 unique computer picked random values
    int rngValue = 0;
    size_t i = 0;

    while (i < COMPUTER_PICK_COUNT) {
        rngValue = RandomNumber::randomNumberGenerator(MIN_PICK_VALUE, MAX_PICK_VALUE);
        if (std::find(m_ComputerPick.begin(), m_ComputerPick.end(), rngValue) == m_ComputerPick2.end()) // Not found
        {
            m_ComputerPick[i] = rngValue;
            i++;
        }
    }
}

The variable type size_t is preferred over int for indexing in containers. Container.size() returns size_t and using int generates warning messages.

Don't Repeat Yourself

This code repeats at least 3 times:

std::cout << `Question` << std::endl;
std::cin >> `Variable`;
std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');

It might be better to write a function that takes an input and returns a string output. It might also be better to use std::getline(std::string)

In software engineering, don't repeat yourself (DRY) is a principle of software development aimed at reducing repetition of software patterns, replacing them with abstractions; and several copies of the same data, using data normalization to avoid redundancy.

Generally when there is repeating code in a software module it indicates that a function should be written to contain that code or a loop should be written to perform the repetition.

When code repeats in different functions it becomes a maintenance problem. Someone can fix the code in one location and miss it in another location. The solution to this is to write a function for the code that repeats.

Reduce Complexity, Follow SRP
The Single Responsibility Principle states that every module or class should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by the class. All its services should be narrowly aligned with that responsibility.

Robert C. Martin expresses the principle as follows: A class should have only one reason to change.

While this is primarily targeted at classes in object oriented languages it applies to functions and subroutines well.

Multiple functions in the program are overly complex and could be broken up. The main() function could be broken up into at multiple functions, especially the outer while loop. The while loop seems to be a good candidate for a function, the two clauses of the if statement also seem to be good candidates for functions.

The more separate functions there are the easier it is to understand or read the code. This also makes it easier for any programmer to maintain or debug the code.

You might also want to look at SOLID programming

Answer 2

It seems to be that you have learned to programme c++ with a book ain't up-to-date. Here are some elements I would improve:

#define

The use of #define in modern C++ is discouraged in favor of constexpr constants. The advantage of this new way is that you can use the same name in multiple scopes. I would suggest to replace them by:

constexpr static auto MIN_PICK_VALUE = 1;

using namespace std

Just remove this line, see Why using namespace is evil.

rand()

The rand() function is deprecated because it doesn't provide you with a random number. See Generating random integer from a range for a correct usage of the <random> header.

GOTO

DONT USE GOTO AT ALL!!! Goto still considered harmful

Just replace that UGLY piece of code with continue instead and forget you ever saw those 4 letters, FAST!

m_playerPick/m_ComputerPick

Try being consistent, both the computer and the player can pick numbers. Just use std::vector<int> for both, it will make your code much more readable.

Unless you have a very, very, very good reason to deviate from that, use std::array<int, COMPUTER_PICK_COUNT> instead. In this case, just use `std::vector, it will make your code easier to understand!

Looking at the KenoWinCalculator, you have written too much code. You can do the same intention with:

KenoWinCalculator(std::vector<int> playerPick) : m_playerPick{std::move(playerPick)} {}
// OR
KenoWinCalculator(const std::vector<int> &playerPick) : m_playerPick{playerPick} {}

generateComputerPick

The code of this function doesn't do what a reader of it expects. You use a while-loop for the for-condition, and a for-loop for a while condition.

I'm assuming std::vector<int>.

void generateComputerPick(){
    m_ComputerPick.clear();
    for (int i = 0; i < COMPUTER_PICK_COUNT; ++i){
        int rngValue = 0;
        do {
            rngValue = RandomNumber::randomNumberGenerator(MIN_PICK_VALUE, MAX_PICK_VALUE);
        while (std::count(m_ComputerPick.cbegin(), m_ComputerPick.cend(), rngValue) != 0);
        m_ComputerPick.push_back(rngValue);
    }

calculateHits()

The code in this function is OK, although I don't like the sizeof in here, again because you can use std::vector<int>. As you are looping over all elements of the container, a ranged-base for-loop would be the better choice:

for (int player : m_playerPickCount)
     if (std::count(m_ComputerPick.cbegin(), m_ComputerPick.cend(), player) != 0)
         ++m_hits;

Alternatively, you could have used set_intersect or similar to reduce the code even more.

Player_pick

The Player_pick and Hit is an enumeration, in this case, just use the numbers.

Conclusion

I haven't read your code completely, though, I believe that fixing the elements above will already make it more readable. Most remarks have to do with elements which are considered bad practice because newer C++ standards provided a better alternative. Something most likely related to the book you are reading more than your effort.

Looking at the code, I can see its written by someone with limited experience, although, I expected a lot worse when I read that this was your first standalone program (in C++?).