A Simple Console Based Keno/Lottery Game in C++
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
4
down vote
favorite
Earlier this week I provided this answer to the Original Poster of the question. Since I've already shown that I am not a C++ Guru, I have made the changes I suggested and I'm posting them for review. My question is what alternative Standard Template Library constructs I could have used to implement this. In case you aren't aware, copying is a major form of flattery.
The Basics of the game are:
The user creates a balance of credits to play the game.
Per game round the user enters a bet.
The user then has the choice of either picking between 5 and 10
numbers between 1 and 60 or allowing the computer to pick between 5
and 10 numbers between 1 and 60.Each game round is scored based on the number of picks and the number
of matches of those picks. Winnings are applied to the balance, bets
reduce the balance.A zero balance prevents game play and allows the user to add credits.
The goals I had when writing this alternate solution are:
Break the one file solution into multiple header and source files to
provide better object oriented programming and reuse.Replace all C programming language constructs with C++ programming.
Improve the readability and maintainability of the code.
Follow the Single Responsibility Principle.
Follow the DRY programming Principle.
Follow the KISS principle.
Reduce the number of lines of code in the solution.
Note: I was not successful in reducing the total number of lines of code, at least partially because breaking the code into header and source files adds some overhead lines of code, and because adding functions adds at least 3 lines of code for every new function. The original solutions is approximately 375 lines of code this alternate solution is approximately 440 lines of code.
One of the few major changes to the code, one is that the function kenoGameplay(Bank &credits)
was turned into a class. The other is that an std::map()
of multiple maps was used to replace a rather large case statement with embedded if statements in Award::awardWon(Bank &bank)
. A third was to decouple the Award
class from the Bank
class by adding another interface to the Bank
class. A fourth was to change a C style array of integers into a std::vector
. The final major change is that the random number generator now returns vectors of unique random numbers rather than a single random number and the interface is no longer a static member.
Possible Bugs in the Original that are Repeated Here
- If this were truly a Keno Game rather than a Lottery Game, then the
bet should be used as a multiplier of the award, there is currently
no reason to bet anything other than 1. - Rather than forcing the user to bet every time allow them to choose
to keep the same bet or change it. - This program still uses the C standard library
rand()
function
rather than more modern random number generators. This is offset
somewhat by the fact that the implementation is now hidden and
changing this will only force relinking after RandomNumber.cpp is
recompiled.
Because I used Visual Studio 2015, this alternate solution is not portable without edits to remove stdafx.h
from source files and replace #pragma once
with guard bandes in the header files.
A specific question about the code is there a single STL construct I can use to replace this function, I don't think std::equal()
will work since one vector is always length 15 and the other will vary between length 5 and length 10.
void KenoWinCalculator::calculateHits()
for each (auto playerPick in m_playerPick)
if (std::find(m_ComputerPick.begin(), m_ComputerPick.end(), playerPick) != m_ComputerPick.end())
m_hits++;
A second specific question I have is would it be better to use std::count()
over std::find()
in the above function.
EDIT
While I am specifically interested in STL improvements, all observations are good, since I'm far from perfect.
GetUserInput.h
#pragma once
#include <iostream>
#include <string>
template<typename T>
T PromptUserReturnResponse(const char* prompt)
T response;
std::cout << prompt << std::endl;
std::cin >> response;
std::cin.ignore(std::numeric_limits<std::streamsize>::max(), 'n');
return response;
kenoGameConstants.h
#pragma once
static const int MIN_PICK_VALUE = 1;
static const int MAX_PICK_VALUE = 60;
static const int COMPUTER_PICK_COUNT = 15;
KenoGame1.cpp
#include "stdafx.h"
#include <iostream>
#include <string>
#include "Bank.h"
#include "KenoGamePlay.h"
#include "GetUserInput.h"
std::string welcomeUser()
std::cout << "******** WELCOME TO WORLD OF KENO ************** n n";
std::string name = PromptUserReturnResponse<decltype(name)>("Please enter your name n");
std::cout << "Hello! " << name << "...nn" << std::endl;
return name;
Bank StartTheBank()
Bank credits;
credits.setBankBalance(PromptUserReturnResponse<int>("Please insert money"));
return credits;
int main()
int programStatus = EXIT_SUCCESS;
std::string name = welcomeUser();
kenoGamePlay kenoGameController(StartTheBank());
programStatus = kenoGameController.GameLoop();
return programStatus;
kenogameplay.h
#pragma once
#include <iostream>
#include <vector>
#include "Bank.h"
//#include "RandomNumber.h"
class kenoGamePlay
private:
Bank &m_credits;
class RandomNumber *m_NumberGenerator;
protected:
int PlayOneGame();
std::vector<int> QuickPickNumbers();
std::vector<int> PlayerPicksNumbers();
bool IsQuickPick();
void ValidateNumberAndAddOrReport(int num, std::vector<int> &gameNumbersToPlay);
void DisplayIntegersInVector(std::vector<int> numbersToPrint) const
for each (auto num in numbersToPrint)
std::cout << num << " ";
std::cout << "n";
public:
kenoGamePlay(Bank &credits);
~kenoGamePlay() = default;
int GameLoop();
;
kenogameplay.cpp
#include "stdafx.h"
#include <iostream>
#include <vector>
#include <string>
#include <algorithm>
#include "kenoGameConstants.h"
#include "kenogameplay.h"
#include "KenoWinCalculator.h"
#include "Award.h"
#include "GetUserInput.h"
#include "RandomNumber.h"
static const size_t QUIT_VALUE = 100;
static const size_t MAX_NOS_PICKED = 10;
static const size_t MIN_NOS_PICKED = 5;
static const size_t INSERT_MONEY = 1;
kenoGamePlay::kenoGamePlay(Bank &credits)
: m_creditscredits
m_NumberGenerator = new RandomNumber();
std::vector<int> kenoGamePlay::QuickPickNumbers()
quickPickCount > MAX_NOS_PICKED);
return m_NumberGenerator->GenerateAndGetVectorOfUniqueRandomNumbers(quickPickCount);
void kenoGamePlay::ValidateNumberAndAddOrReport(int num, std::vector<int> &gameNumbersToPlay)
if (num >= MIN_PICK_VALUE && num <= MAX_PICK_VALUE)
if (std::count(gameNumbersToPlay.begin(), gameNumbersToPlay.end(), num) != 0)
std::cout << "You already picked " << num << ". Please select another number. n";
else
gameNumbersToPlay.push_back(num);
else
std::cout << "You entered an invalid number. Please try again n";
std::vector<int> kenoGamePlay::PlayerPicksNumbers()
std::cout << "Pick between " << MIN_NOS_PICKED << " and " << MAX_NOS_PICKED <<
" unique numbers between " << MIN_PICK_VALUE << " and " << MAX_PICK_VALUE <<
". Enter " << QUIT_VALUE << " to QUIT entering numbersn";
int num = 0;
std::vector<int> gameNumbersToPlay;
while (gameNumbersToPlay.size() < MAX_NOS_PICKED)
num = PromptUserReturnResponse<decltype(num)>("Enter a number>>");
if (num == QUIT_VALUE)
if (gameNumbersToPlay.size() >= MIN_NOS_PICKED)
return gameNumbersToPlay;
else
std::cout << "You haven't selected enough numbers yet, please pick more numbers.n";
else
ValidateNumberAndAddOrReport(num, gameNumbersToPlay);
return gameNumbersToPlay;
int kenoGamePlay::GameLoop()
int programStatus = EXIT_SUCCESS;
char continueGameplay = 'y';
bool isBetInvalid = true;
int insertMoneyReduceBet = 0;
while (programStatus == EXIT_SUCCESS && continueGameplay != 'n')
if (m_credits.isEnoughBalanceToPlayGame()
return programStatus;
int kenoGamePlay::PlayOneGame()
int programStatus = EXIT_SUCCESS;
std::vector<int> nosPicked = (IsQuickPick()) ? QuickPickNumbers() : PlayerPicksNumbers();
KenoWinCalculator winCalc(nosPicked);
winCalc.generateComputerPick(m_NumberGenerator);
winCalc.calculateHits();
Award wonAward(winCalc);
wonAward.awardWon(m_credits);
std::vector<int> playerPickedValues = winCalc.getPlayerPickValues();
std::cout << "Your selection: t";
DisplayIntegersInVector(playerPickedValues);
std::cout << "nComputer selection: t";
DisplayIntegersInVector(winCalc.getComputerPick());
std::cout << "nno of hits: " << winCalc.getHitValue() << "n" << "Your bank balance is: $" << m_credits.getBankBalance() << "n";
std::cout.flush();
return programStatus;
bool kenoGamePlay::IsQuickPick()
return (PromptUserReturnResponse<char>("Do you want the computer to pick the numbers for you? [y]es or [n]o") == 'y');
Bank.h
#pragma once
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
~Bank() = default;
void setBet(int betValue) m_bet = betValue;
void setBankBalance(int balance) m_bankBalance += balance;
void updatebankBalance() m_bankBalance -= m_bet;
void depositWinnings(int winnings) m_bankBalance += winnings;
int getBankBalance() const return m_bankBalance;
bool isEnoughBalanceToPlayGame() const return m_bankBalance >= m_bet;
bool isBetValid() const return m_bet <= m_bankBalance && m_bet > 0;
;
RandomNumber.h
#pragma once
#include <vector>
class RandomNumber // Random number generator
private:
std::vector<int> UniqueRandomNumbersContainer;
protected:
int randomNumberGenerator();
public:
RandomNumber();
~RandomNumber();
void resetUniqueRandomNumbersContainer()
if (!UniqueRandomNumbersContainer.empty())
UniqueRandomNumbersContainer.clear();
std::vector<int> getaVectorOfUniqueRandomNumbers() const return UniqueRandomNumbersContainer;
std::vector<int> GenerateAndGetVectorOfUniqueRandomNumbers(size_t UniqueRandomNumberCount)
generateUniqueRandomNumbers(UniqueRandomNumberCount);
return UniqueRandomNumbersContainer;
void generateUniqueRandomNumbers(size_t UniqueRandomNumberCount);
;
RandomNumber.cpp
#include <ctime>
#include <algorithm>
#include "stdafx.h"
#include "RandomNumber.h"
#include "kenoGameConstants.h"
RandomNumber::RandomNumber()
srand(static_cast<unsigned int>(time(0)));
RandomNumber::~RandomNumber()
resetUniqueRandomNumbersContainer();
int RandomNumber::randomNumberGenerator()
int randomRange = MIN_PICK_VALUE + MAX_PICK_VALUE;
return (rand() % randomRange);
void RandomNumber::generateUniqueRandomNumbers(size_t UniqueRandomNumberCount)
if (!UniqueRandomNumbersContainer.empty())
UniqueRandomNumbersContainer.clear();
size_t generatedCount = 0;
while (generatedCount < UniqueRandomNumberCount)
int rngValue = randomNumberGenerator();
if (std::find(UniqueRandomNumbersContainer.begin(), UniqueRandomNumbersContainer.end(), rngValue) == UniqueRandomNumbersContainer.end())
UniqueRandomNumbersContainer.push_back(rngValue);
generatedCount++;
award.h
#pragma once
#include "KenoWinCalculator.h"
#include "Bank.h"
#include <map>
class Award
private: // This class maintains pay table
typedef std::map<int, int> HitMap;
typedef std::map<int, HitMap> PlayerPickMap;
int m_picks;
int m_hits;
PlayerPickMap MapSelector;
public:
Award(const KenoWinCalculator &calc);
~Award();
void awardWon(Bank &bank);
;
Award.cpp
#include <iostream>
#include "award.h"
#include "KenoWinCalculator.h"
Award::Award(const KenoWinCalculator &calc)
m_picks = calc.getPlayerPickCount();
m_hits = calc.getHitValue();
// In each of the follow tuples the first number is the number of hits or matches
// and the second number is the dollar reward. ie 4, 50 means 4 hits and $50.
HitMap Pick5Hits = 4, 50, 5, 100 ;
HitMap Pick6Hits = 4, 100, 5, 120, 6, 150 ;
HitMap Pick7Hits = 3, 80, 4, 90, 5, 100, 6, 120, 7, 150 ;
HitMap Pick8Hits = 3, 80, 4, 90, 5, 100, 6, 120, 7, 150, 8, 175 ;
HitMap Pick9Hits = 3, 80, 4, 90, 5, 100, 6, 120, 7, 150, 8, 175, 9, 200 ;
HitMap Pick10Hits = 3, 80, 4, 90, 5, 100, 6, 120, 7, 150, 8, 175, 9, 200, 10, 250 ;
MapSelector = 5, Pick5Hits, 6, Pick6Hits, 7, Pick7Hits, 8, Pick8Hits, 9, Pick9Hits, 10, Pick10Hits ;
Award::~Award()
for each(auto hitmap in MapSelector)
hitmap.second.clear();
MapSelector.clear();
void Award::awardWon(Bank &bank)
int amtWon = 0;
auto correctHitMap = MapSelector.find(m_picks);
if (correctHitMap != MapSelector.end())
HitMap HitMapTosearch = correctHitMap->second;
auto HitValueMap = HitMapTosearch.find(m_hits);
if (HitValueMap != HitMapTosearch.end())
amtWon = HitValueMap->second;
if (amtWon > 0)
std::cout << "Congratulations you won $" << amtWon << "!n";
bank.depositWinnings(amtWon);
else
std::cout << "Sorry you lostn";
std::cout << std::endl; // flush the output
KenoWinCalculator.h
#pragma once
#include <vector>
#include "RandomNumber.h"
class KenoWinCalculator
private:
std::vector<int> m_playerPick; // vector is used because the size is not fixed,
std::vector<int> m_ComputerPick; // player can choose any picks between 5 and 10 but computer's is fixed
int m_hits = 0;
public:
KenoWinCalculator(const std::vector<int> &playerPick);
~KenoWinCalculator() = default;
void generateComputerPick(RandomNumber* NumberGenerator); // Generate COMPUTER_PICK_COUNT unique computer picked random values
void calculateHits(); // Calculates no of common nos between player and computer pick
int getHitValue() const return m_hits;
int getPlayerPickCount() const return static_cast<int>(m_playerPick.size());
const std::vector<int>& getPlayerPickValues() const return m_playerPick;
std::vector<int> getComputerPick() const return m_ComputerPick;
;
KenoWinCalculator.cpp
#include <algorithm>
#include "kenoGameConstants.h"
#include "RandomNumber.h"
#include "KenoWinCalculator.h"
KenoWinCalculator::KenoWinCalculator(const std::vector<int> &playerPick)
m_playerPick = playerPick;
void KenoWinCalculator::generateComputerPick(RandomNumber* NumberGenerator) // Generate 10 unique computer picked random values
m_ComputerPick = NumberGenerator->GenerateAndGetVectorOfUniqueRandomNumbers(COMPUTER_PICK_COUNT);
void KenoWinCalculator::calculateHits()
for each (auto playerPick in m_playerPick)
if (std::find(m_ComputerPick.begin(), m_ComputerPick.end(), playerPick) != m_ComputerPick.end())
m_hits++;
c++ c++11 game
 |Â
show 1 more comment
up vote
4
down vote
favorite
Earlier this week I provided this answer to the Original Poster of the question. Since I've already shown that I am not a C++ Guru, I have made the changes I suggested and I'm posting them for review. My question is what alternative Standard Template Library constructs I could have used to implement this. In case you aren't aware, copying is a major form of flattery.
The Basics of the game are:
The user creates a balance of credits to play the game.
Per game round the user enters a bet.
The user then has the choice of either picking between 5 and 10
numbers between 1 and 60 or allowing the computer to pick between 5
and 10 numbers between 1 and 60.Each game round is scored based on the number of picks and the number
of matches of those picks. Winnings are applied to the balance, bets
reduce the balance.A zero balance prevents game play and allows the user to add credits.
The goals I had when writing this alternate solution are:
Break the one file solution into multiple header and source files to
provide better object oriented programming and reuse.Replace all C programming language constructs with C++ programming.
Improve the readability and maintainability of the code.
Follow the Single Responsibility Principle.
Follow the DRY programming Principle.
Follow the KISS principle.
Reduce the number of lines of code in the solution.
Note: I was not successful in reducing the total number of lines of code, at least partially because breaking the code into header and source files adds some overhead lines of code, and because adding functions adds at least 3 lines of code for every new function. The original solutions is approximately 375 lines of code this alternate solution is approximately 440 lines of code.
One of the few major changes to the code, one is that the function kenoGameplay(Bank &credits)
was turned into a class. The other is that an std::map()
of multiple maps was used to replace a rather large case statement with embedded if statements in Award::awardWon(Bank &bank)
. A third was to decouple the Award
class from the Bank
class by adding another interface to the Bank
class. A fourth was to change a C style array of integers into a std::vector
. The final major change is that the random number generator now returns vectors of unique random numbers rather than a single random number and the interface is no longer a static member.
Possible Bugs in the Original that are Repeated Here
- If this were truly a Keno Game rather than a Lottery Game, then the
bet should be used as a multiplier of the award, there is currently
no reason to bet anything other than 1. - Rather than forcing the user to bet every time allow them to choose
to keep the same bet or change it. - This program still uses the C standard library
rand()
function
rather than more modern random number generators. This is offset
somewhat by the fact that the implementation is now hidden and
changing this will only force relinking after RandomNumber.cpp is
recompiled.
Because I used Visual Studio 2015, this alternate solution is not portable without edits to remove stdafx.h
from source files and replace #pragma once
with guard bandes in the header files.
A specific question about the code is there a single STL construct I can use to replace this function, I don't think std::equal()
will work since one vector is always length 15 and the other will vary between length 5 and length 10.
void KenoWinCalculator::calculateHits()
for each (auto playerPick in m_playerPick)
if (std::find(m_ComputerPick.begin(), m_ComputerPick.end(), playerPick) != m_ComputerPick.end())
m_hits++;
A second specific question I have is would it be better to use std::count()
over std::find()
in the above function.
EDIT
While I am specifically interested in STL improvements, all observations are good, since I'm far from perfect.
GetUserInput.h
#pragma once
#include <iostream>
#include <string>
template<typename T>
T PromptUserReturnResponse(const char* prompt)
T response;
std::cout << prompt << std::endl;
std::cin >> response;
std::cin.ignore(std::numeric_limits<std::streamsize>::max(), 'n');
return response;
kenoGameConstants.h
#pragma once
static const int MIN_PICK_VALUE = 1;
static const int MAX_PICK_VALUE = 60;
static const int COMPUTER_PICK_COUNT = 15;
KenoGame1.cpp
#include "stdafx.h"
#include <iostream>
#include <string>
#include "Bank.h"
#include "KenoGamePlay.h"
#include "GetUserInput.h"
std::string welcomeUser()
std::cout << "******** WELCOME TO WORLD OF KENO ************** n n";
std::string name = PromptUserReturnResponse<decltype(name)>("Please enter your name n");
std::cout << "Hello! " << name << "...nn" << std::endl;
return name;
Bank StartTheBank()
Bank credits;
credits.setBankBalance(PromptUserReturnResponse<int>("Please insert money"));
return credits;
int main()
int programStatus = EXIT_SUCCESS;
std::string name = welcomeUser();
kenoGamePlay kenoGameController(StartTheBank());
programStatus = kenoGameController.GameLoop();
return programStatus;
kenogameplay.h
#pragma once
#include <iostream>
#include <vector>
#include "Bank.h"
//#include "RandomNumber.h"
class kenoGamePlay
private:
Bank &m_credits;
class RandomNumber *m_NumberGenerator;
protected:
int PlayOneGame();
std::vector<int> QuickPickNumbers();
std::vector<int> PlayerPicksNumbers();
bool IsQuickPick();
void ValidateNumberAndAddOrReport(int num, std::vector<int> &gameNumbersToPlay);
void DisplayIntegersInVector(std::vector<int> numbersToPrint) const
for each (auto num in numbersToPrint)
std::cout << num << " ";
std::cout << "n";
public:
kenoGamePlay(Bank &credits);
~kenoGamePlay() = default;
int GameLoop();
;
kenogameplay.cpp
#include "stdafx.h"
#include <iostream>
#include <vector>
#include <string>
#include <algorithm>
#include "kenoGameConstants.h"
#include "kenogameplay.h"
#include "KenoWinCalculator.h"
#include "Award.h"
#include "GetUserInput.h"
#include "RandomNumber.h"
static const size_t QUIT_VALUE = 100;
static const size_t MAX_NOS_PICKED = 10;
static const size_t MIN_NOS_PICKED = 5;
static const size_t INSERT_MONEY = 1;
kenoGamePlay::kenoGamePlay(Bank &credits)
: m_creditscredits
m_NumberGenerator = new RandomNumber();
std::vector<int> kenoGamePlay::QuickPickNumbers()
quickPickCount > MAX_NOS_PICKED);
return m_NumberGenerator->GenerateAndGetVectorOfUniqueRandomNumbers(quickPickCount);
void kenoGamePlay::ValidateNumberAndAddOrReport(int num, std::vector<int> &gameNumbersToPlay)
if (num >= MIN_PICK_VALUE && num <= MAX_PICK_VALUE)
if (std::count(gameNumbersToPlay.begin(), gameNumbersToPlay.end(), num) != 0)
std::cout << "You already picked " << num << ". Please select another number. n";
else
gameNumbersToPlay.push_back(num);
else
std::cout << "You entered an invalid number. Please try again n";
std::vector<int> kenoGamePlay::PlayerPicksNumbers()
std::cout << "Pick between " << MIN_NOS_PICKED << " and " << MAX_NOS_PICKED <<
" unique numbers between " << MIN_PICK_VALUE << " and " << MAX_PICK_VALUE <<
". Enter " << QUIT_VALUE << " to QUIT entering numbersn";
int num = 0;
std::vector<int> gameNumbersToPlay;
while (gameNumbersToPlay.size() < MAX_NOS_PICKED)
num = PromptUserReturnResponse<decltype(num)>("Enter a number>>");
if (num == QUIT_VALUE)
if (gameNumbersToPlay.size() >= MIN_NOS_PICKED)
return gameNumbersToPlay;
else
std::cout << "You haven't selected enough numbers yet, please pick more numbers.n";
else
ValidateNumberAndAddOrReport(num, gameNumbersToPlay);
return gameNumbersToPlay;
int kenoGamePlay::GameLoop()
int programStatus = EXIT_SUCCESS;
char continueGameplay = 'y';
bool isBetInvalid = true;
int insertMoneyReduceBet = 0;
while (programStatus == EXIT_SUCCESS && continueGameplay != 'n')
if (m_credits.isEnoughBalanceToPlayGame()
return programStatus;
int kenoGamePlay::PlayOneGame()
int programStatus = EXIT_SUCCESS;
std::vector<int> nosPicked = (IsQuickPick()) ? QuickPickNumbers() : PlayerPicksNumbers();
KenoWinCalculator winCalc(nosPicked);
winCalc.generateComputerPick(m_NumberGenerator);
winCalc.calculateHits();
Award wonAward(winCalc);
wonAward.awardWon(m_credits);
std::vector<int> playerPickedValues = winCalc.getPlayerPickValues();
std::cout << "Your selection: t";
DisplayIntegersInVector(playerPickedValues);
std::cout << "nComputer selection: t";
DisplayIntegersInVector(winCalc.getComputerPick());
std::cout << "nno of hits: " << winCalc.getHitValue() << "n" << "Your bank balance is: $" << m_credits.getBankBalance() << "n";
std::cout.flush();
return programStatus;
bool kenoGamePlay::IsQuickPick()
return (PromptUserReturnResponse<char>("Do you want the computer to pick the numbers for you? [y]es or [n]o") == 'y');
Bank.h
#pragma once
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
~Bank() = default;
void setBet(int betValue) m_bet = betValue;
void setBankBalance(int balance) m_bankBalance += balance;
void updatebankBalance() m_bankBalance -= m_bet;
void depositWinnings(int winnings) m_bankBalance += winnings;
int getBankBalance() const return m_bankBalance;
bool isEnoughBalanceToPlayGame() const return m_bankBalance >= m_bet;
bool isBetValid() const return m_bet <= m_bankBalance && m_bet > 0;
;
RandomNumber.h
#pragma once
#include <vector>
class RandomNumber // Random number generator
private:
std::vector<int> UniqueRandomNumbersContainer;
protected:
int randomNumberGenerator();
public:
RandomNumber();
~RandomNumber();
void resetUniqueRandomNumbersContainer()
if (!UniqueRandomNumbersContainer.empty())
UniqueRandomNumbersContainer.clear();
std::vector<int> getaVectorOfUniqueRandomNumbers() const return UniqueRandomNumbersContainer;
std::vector<int> GenerateAndGetVectorOfUniqueRandomNumbers(size_t UniqueRandomNumberCount)
generateUniqueRandomNumbers(UniqueRandomNumberCount);
return UniqueRandomNumbersContainer;
void generateUniqueRandomNumbers(size_t UniqueRandomNumberCount);
;
RandomNumber.cpp
#include <ctime>
#include <algorithm>
#include "stdafx.h"
#include "RandomNumber.h"
#include "kenoGameConstants.h"
RandomNumber::RandomNumber()
srand(static_cast<unsigned int>(time(0)));
RandomNumber::~RandomNumber()
resetUniqueRandomNumbersContainer();
int RandomNumber::randomNumberGenerator()
int randomRange = MIN_PICK_VALUE + MAX_PICK_VALUE;
return (rand() % randomRange);
void RandomNumber::generateUniqueRandomNumbers(size_t UniqueRandomNumberCount)
if (!UniqueRandomNumbersContainer.empty())
UniqueRandomNumbersContainer.clear();
size_t generatedCount = 0;
while (generatedCount < UniqueRandomNumberCount)
int rngValue = randomNumberGenerator();
if (std::find(UniqueRandomNumbersContainer.begin(), UniqueRandomNumbersContainer.end(), rngValue) == UniqueRandomNumbersContainer.end())
UniqueRandomNumbersContainer.push_back(rngValue);
generatedCount++;
award.h
#pragma once
#include "KenoWinCalculator.h"
#include "Bank.h"
#include <map>
class Award
private: // This class maintains pay table
typedef std::map<int, int> HitMap;
typedef std::map<int, HitMap> PlayerPickMap;
int m_picks;
int m_hits;
PlayerPickMap MapSelector;
public:
Award(const KenoWinCalculator &calc);
~Award();
void awardWon(Bank &bank);
;
Award.cpp
#include <iostream>
#include "award.h"
#include "KenoWinCalculator.h"
Award::Award(const KenoWinCalculator &calc)
m_picks = calc.getPlayerPickCount();
m_hits = calc.getHitValue();
// In each of the follow tuples the first number is the number of hits or matches
// and the second number is the dollar reward. ie 4, 50 means 4 hits and $50.
HitMap Pick5Hits = 4, 50, 5, 100 ;
HitMap Pick6Hits = 4, 100, 5, 120, 6, 150 ;
HitMap Pick7Hits = 3, 80, 4, 90, 5, 100, 6, 120, 7, 150 ;
HitMap Pick8Hits = 3, 80, 4, 90, 5, 100, 6, 120, 7, 150, 8, 175 ;
HitMap Pick9Hits = 3, 80, 4, 90, 5, 100, 6, 120, 7, 150, 8, 175, 9, 200 ;
HitMap Pick10Hits = 3, 80, 4, 90, 5, 100, 6, 120, 7, 150, 8, 175, 9, 200, 10, 250 ;
MapSelector = 5, Pick5Hits, 6, Pick6Hits, 7, Pick7Hits, 8, Pick8Hits, 9, Pick9Hits, 10, Pick10Hits ;
Award::~Award()
for each(auto hitmap in MapSelector)
hitmap.second.clear();
MapSelector.clear();
void Award::awardWon(Bank &bank)
int amtWon = 0;
auto correctHitMap = MapSelector.find(m_picks);
if (correctHitMap != MapSelector.end())
HitMap HitMapTosearch = correctHitMap->second;
auto HitValueMap = HitMapTosearch.find(m_hits);
if (HitValueMap != HitMapTosearch.end())
amtWon = HitValueMap->second;
if (amtWon > 0)
std::cout << "Congratulations you won $" << amtWon << "!n";
bank.depositWinnings(amtWon);
else
std::cout << "Sorry you lostn";
std::cout << std::endl; // flush the output
KenoWinCalculator.h
#pragma once
#include <vector>
#include "RandomNumber.h"
class KenoWinCalculator
private:
std::vector<int> m_playerPick; // vector is used because the size is not fixed,
std::vector<int> m_ComputerPick; // player can choose any picks between 5 and 10 but computer's is fixed
int m_hits = 0;
public:
KenoWinCalculator(const std::vector<int> &playerPick);
~KenoWinCalculator() = default;
void generateComputerPick(RandomNumber* NumberGenerator); // Generate COMPUTER_PICK_COUNT unique computer picked random values
void calculateHits(); // Calculates no of common nos between player and computer pick
int getHitValue() const return m_hits;
int getPlayerPickCount() const return static_cast<int>(m_playerPick.size());
const std::vector<int>& getPlayerPickValues() const return m_playerPick;
std::vector<int> getComputerPick() const return m_ComputerPick;
;
KenoWinCalculator.cpp
#include <algorithm>
#include "kenoGameConstants.h"
#include "RandomNumber.h"
#include "KenoWinCalculator.h"
KenoWinCalculator::KenoWinCalculator(const std::vector<int> &playerPick)
m_playerPick = playerPick;
void KenoWinCalculator::generateComputerPick(RandomNumber* NumberGenerator) // Generate 10 unique computer picked random values
m_ComputerPick = NumberGenerator->GenerateAndGetVectorOfUniqueRandomNumbers(COMPUTER_PICK_COUNT);
void KenoWinCalculator::calculateHits()
for each (auto playerPick in m_playerPick)
if (std::find(m_ComputerPick.begin(), m_ComputerPick.end(), playerPick) != m_ComputerPick.end())
m_hits++;
c++ c++11 game
Just to make sure, are you only interested in improvements regarding STL usage or would you also like general hints and corrections?
â Ben Steffan
Jan 5 at 20:08
@BenSteffan General hints and suggestions are good, but I am interested in the STL usage. I'm aware that I could improve my naming conventions, the use of Camel Case versus First Cap is inconsistent. Feel free to review everything since it is a learning experience no matter what.
â pacmaninbw
Jan 5 at 20:14
1
Fine, then I'll go ahead.
â Ben Steffan
Jan 5 at 20:15
Does your program actually compile? A conforming compiler should reject it.
â Ben Steffan
Jan 5 at 21:14
1
I think I remember that Visual Studio allowed that one to slip. Give me 5 minutes, I'm nearly done with my answer that includes a description of what I am talking about.
â Ben Steffan
Jan 5 at 21:32
 |Â
show 1 more comment
up vote
4
down vote
favorite
up vote
4
down vote
favorite
Earlier this week I provided this answer to the Original Poster of the question. Since I've already shown that I am not a C++ Guru, I have made the changes I suggested and I'm posting them for review. My question is what alternative Standard Template Library constructs I could have used to implement this. In case you aren't aware, copying is a major form of flattery.
The Basics of the game are:
The user creates a balance of credits to play the game.
Per game round the user enters a bet.
The user then has the choice of either picking between 5 and 10
numbers between 1 and 60 or allowing the computer to pick between 5
and 10 numbers between 1 and 60.Each game round is scored based on the number of picks and the number
of matches of those picks. Winnings are applied to the balance, bets
reduce the balance.A zero balance prevents game play and allows the user to add credits.
The goals I had when writing this alternate solution are:
Break the one file solution into multiple header and source files to
provide better object oriented programming and reuse.Replace all C programming language constructs with C++ programming.
Improve the readability and maintainability of the code.
Follow the Single Responsibility Principle.
Follow the DRY programming Principle.
Follow the KISS principle.
Reduce the number of lines of code in the solution.
Note: I was not successful in reducing the total number of lines of code, at least partially because breaking the code into header and source files adds some overhead lines of code, and because adding functions adds at least 3 lines of code for every new function. The original solutions is approximately 375 lines of code this alternate solution is approximately 440 lines of code.
One of the few major changes to the code, one is that the function kenoGameplay(Bank &credits)
was turned into a class. The other is that an std::map()
of multiple maps was used to replace a rather large case statement with embedded if statements in Award::awardWon(Bank &bank)
. A third was to decouple the Award
class from the Bank
class by adding another interface to the Bank
class. A fourth was to change a C style array of integers into a std::vector
. The final major change is that the random number generator now returns vectors of unique random numbers rather than a single random number and the interface is no longer a static member.
Possible Bugs in the Original that are Repeated Here
- If this were truly a Keno Game rather than a Lottery Game, then the
bet should be used as a multiplier of the award, there is currently
no reason to bet anything other than 1. - Rather than forcing the user to bet every time allow them to choose
to keep the same bet or change it. - This program still uses the C standard library
rand()
function
rather than more modern random number generators. This is offset
somewhat by the fact that the implementation is now hidden and
changing this will only force relinking after RandomNumber.cpp is
recompiled.
Because I used Visual Studio 2015, this alternate solution is not portable without edits to remove stdafx.h
from source files and replace #pragma once
with guard bandes in the header files.
A specific question about the code is there a single STL construct I can use to replace this function, I don't think std::equal()
will work since one vector is always length 15 and the other will vary between length 5 and length 10.
void KenoWinCalculator::calculateHits()
for each (auto playerPick in m_playerPick)
if (std::find(m_ComputerPick.begin(), m_ComputerPick.end(), playerPick) != m_ComputerPick.end())
m_hits++;
A second specific question I have is would it be better to use std::count()
over std::find()
in the above function.
EDIT
While I am specifically interested in STL improvements, all observations are good, since I'm far from perfect.
GetUserInput.h
#pragma once
#include <iostream>
#include <string>
template<typename T>
T PromptUserReturnResponse(const char* prompt)
T response;
std::cout << prompt << std::endl;
std::cin >> response;
std::cin.ignore(std::numeric_limits<std::streamsize>::max(), 'n');
return response;
kenoGameConstants.h
#pragma once
static const int MIN_PICK_VALUE = 1;
static const int MAX_PICK_VALUE = 60;
static const int COMPUTER_PICK_COUNT = 15;
KenoGame1.cpp
#include "stdafx.h"
#include <iostream>
#include <string>
#include "Bank.h"
#include "KenoGamePlay.h"
#include "GetUserInput.h"
std::string welcomeUser()
std::cout << "******** WELCOME TO WORLD OF KENO ************** n n";
std::string name = PromptUserReturnResponse<decltype(name)>("Please enter your name n");
std::cout << "Hello! " << name << "...nn" << std::endl;
return name;
Bank StartTheBank()
Bank credits;
credits.setBankBalance(PromptUserReturnResponse<int>("Please insert money"));
return credits;
int main()
int programStatus = EXIT_SUCCESS;
std::string name = welcomeUser();
kenoGamePlay kenoGameController(StartTheBank());
programStatus = kenoGameController.GameLoop();
return programStatus;
kenogameplay.h
#pragma once
#include <iostream>
#include <vector>
#include "Bank.h"
//#include "RandomNumber.h"
class kenoGamePlay
private:
Bank &m_credits;
class RandomNumber *m_NumberGenerator;
protected:
int PlayOneGame();
std::vector<int> QuickPickNumbers();
std::vector<int> PlayerPicksNumbers();
bool IsQuickPick();
void ValidateNumberAndAddOrReport(int num, std::vector<int> &gameNumbersToPlay);
void DisplayIntegersInVector(std::vector<int> numbersToPrint) const
for each (auto num in numbersToPrint)
std::cout << num << " ";
std::cout << "n";
public:
kenoGamePlay(Bank &credits);
~kenoGamePlay() = default;
int GameLoop();
;
kenogameplay.cpp
#include "stdafx.h"
#include <iostream>
#include <vector>
#include <string>
#include <algorithm>
#include "kenoGameConstants.h"
#include "kenogameplay.h"
#include "KenoWinCalculator.h"
#include "Award.h"
#include "GetUserInput.h"
#include "RandomNumber.h"
static const size_t QUIT_VALUE = 100;
static const size_t MAX_NOS_PICKED = 10;
static const size_t MIN_NOS_PICKED = 5;
static const size_t INSERT_MONEY = 1;
kenoGamePlay::kenoGamePlay(Bank &credits)
: m_creditscredits
m_NumberGenerator = new RandomNumber();
std::vector<int> kenoGamePlay::QuickPickNumbers()
quickPickCount > MAX_NOS_PICKED);
return m_NumberGenerator->GenerateAndGetVectorOfUniqueRandomNumbers(quickPickCount);
void kenoGamePlay::ValidateNumberAndAddOrReport(int num, std::vector<int> &gameNumbersToPlay)
if (num >= MIN_PICK_VALUE && num <= MAX_PICK_VALUE)
if (std::count(gameNumbersToPlay.begin(), gameNumbersToPlay.end(), num) != 0)
std::cout << "You already picked " << num << ". Please select another number. n";
else
gameNumbersToPlay.push_back(num);
else
std::cout << "You entered an invalid number. Please try again n";
std::vector<int> kenoGamePlay::PlayerPicksNumbers()
std::cout << "Pick between " << MIN_NOS_PICKED << " and " << MAX_NOS_PICKED <<
" unique numbers between " << MIN_PICK_VALUE << " and " << MAX_PICK_VALUE <<
". Enter " << QUIT_VALUE << " to QUIT entering numbersn";
int num = 0;
std::vector<int> gameNumbersToPlay;
while (gameNumbersToPlay.size() < MAX_NOS_PICKED)
num = PromptUserReturnResponse<decltype(num)>("Enter a number>>");
if (num == QUIT_VALUE)
if (gameNumbersToPlay.size() >= MIN_NOS_PICKED)
return gameNumbersToPlay;
else
std::cout << "You haven't selected enough numbers yet, please pick more numbers.n";
else
ValidateNumberAndAddOrReport(num, gameNumbersToPlay);
return gameNumbersToPlay;
int kenoGamePlay::GameLoop()
int programStatus = EXIT_SUCCESS;
char continueGameplay = 'y';
bool isBetInvalid = true;
int insertMoneyReduceBet = 0;
while (programStatus == EXIT_SUCCESS && continueGameplay != 'n')
if (m_credits.isEnoughBalanceToPlayGame()
return programStatus;
int kenoGamePlay::PlayOneGame()
int programStatus = EXIT_SUCCESS;
std::vector<int> nosPicked = (IsQuickPick()) ? QuickPickNumbers() : PlayerPicksNumbers();
KenoWinCalculator winCalc(nosPicked);
winCalc.generateComputerPick(m_NumberGenerator);
winCalc.calculateHits();
Award wonAward(winCalc);
wonAward.awardWon(m_credits);
std::vector<int> playerPickedValues = winCalc.getPlayerPickValues();
std::cout << "Your selection: t";
DisplayIntegersInVector(playerPickedValues);
std::cout << "nComputer selection: t";
DisplayIntegersInVector(winCalc.getComputerPick());
std::cout << "nno of hits: " << winCalc.getHitValue() << "n" << "Your bank balance is: $" << m_credits.getBankBalance() << "n";
std::cout.flush();
return programStatus;
bool kenoGamePlay::IsQuickPick()
return (PromptUserReturnResponse<char>("Do you want the computer to pick the numbers for you? [y]es or [n]o") == 'y');
Bank.h
#pragma once
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
~Bank() = default;
void setBet(int betValue) m_bet = betValue;
void setBankBalance(int balance) m_bankBalance += balance;
void updatebankBalance() m_bankBalance -= m_bet;
void depositWinnings(int winnings) m_bankBalance += winnings;
int getBankBalance() const return m_bankBalance;
bool isEnoughBalanceToPlayGame() const return m_bankBalance >= m_bet;
bool isBetValid() const return m_bet <= m_bankBalance && m_bet > 0;
;
RandomNumber.h
#pragma once
#include <vector>
class RandomNumber // Random number generator
private:
std::vector<int> UniqueRandomNumbersContainer;
protected:
int randomNumberGenerator();
public:
RandomNumber();
~RandomNumber();
void resetUniqueRandomNumbersContainer()
if (!UniqueRandomNumbersContainer.empty())
UniqueRandomNumbersContainer.clear();
std::vector<int> getaVectorOfUniqueRandomNumbers() const return UniqueRandomNumbersContainer;
std::vector<int> GenerateAndGetVectorOfUniqueRandomNumbers(size_t UniqueRandomNumberCount)
generateUniqueRandomNumbers(UniqueRandomNumberCount);
return UniqueRandomNumbersContainer;
void generateUniqueRandomNumbers(size_t UniqueRandomNumberCount);
;
RandomNumber.cpp
#include <ctime>
#include <algorithm>
#include "stdafx.h"
#include "RandomNumber.h"
#include "kenoGameConstants.h"
RandomNumber::RandomNumber()
srand(static_cast<unsigned int>(time(0)));
RandomNumber::~RandomNumber()
resetUniqueRandomNumbersContainer();
int RandomNumber::randomNumberGenerator()
int randomRange = MIN_PICK_VALUE + MAX_PICK_VALUE;
return (rand() % randomRange);
void RandomNumber::generateUniqueRandomNumbers(size_t UniqueRandomNumberCount)
if (!UniqueRandomNumbersContainer.empty())
UniqueRandomNumbersContainer.clear();
size_t generatedCount = 0;
while (generatedCount < UniqueRandomNumberCount)
int rngValue = randomNumberGenerator();
if (std::find(UniqueRandomNumbersContainer.begin(), UniqueRandomNumbersContainer.end(), rngValue) == UniqueRandomNumbersContainer.end())
UniqueRandomNumbersContainer.push_back(rngValue);
generatedCount++;
award.h
#pragma once
#include "KenoWinCalculator.h"
#include "Bank.h"
#include <map>
class Award
private: // This class maintains pay table
typedef std::map<int, int> HitMap;
typedef std::map<int, HitMap> PlayerPickMap;
int m_picks;
int m_hits;
PlayerPickMap MapSelector;
public:
Award(const KenoWinCalculator &calc);
~Award();
void awardWon(Bank &bank);
;
Award.cpp
#include <iostream>
#include "award.h"
#include "KenoWinCalculator.h"
Award::Award(const KenoWinCalculator &calc)
m_picks = calc.getPlayerPickCount();
m_hits = calc.getHitValue();
// In each of the follow tuples the first number is the number of hits or matches
// and the second number is the dollar reward. ie 4, 50 means 4 hits and $50.
HitMap Pick5Hits = 4, 50, 5, 100 ;
HitMap Pick6Hits = 4, 100, 5, 120, 6, 150 ;
HitMap Pick7Hits = 3, 80, 4, 90, 5, 100, 6, 120, 7, 150 ;
HitMap Pick8Hits = 3, 80, 4, 90, 5, 100, 6, 120, 7, 150, 8, 175 ;
HitMap Pick9Hits = 3, 80, 4, 90, 5, 100, 6, 120, 7, 150, 8, 175, 9, 200 ;
HitMap Pick10Hits = 3, 80, 4, 90, 5, 100, 6, 120, 7, 150, 8, 175, 9, 200, 10, 250 ;
MapSelector = 5, Pick5Hits, 6, Pick6Hits, 7, Pick7Hits, 8, Pick8Hits, 9, Pick9Hits, 10, Pick10Hits ;
Award::~Award()
for each(auto hitmap in MapSelector)
hitmap.second.clear();
MapSelector.clear();
void Award::awardWon(Bank &bank)
int amtWon = 0;
auto correctHitMap = MapSelector.find(m_picks);
if (correctHitMap != MapSelector.end())
HitMap HitMapTosearch = correctHitMap->second;
auto HitValueMap = HitMapTosearch.find(m_hits);
if (HitValueMap != HitMapTosearch.end())
amtWon = HitValueMap->second;
if (amtWon > 0)
std::cout << "Congratulations you won $" << amtWon << "!n";
bank.depositWinnings(amtWon);
else
std::cout << "Sorry you lostn";
std::cout << std::endl; // flush the output
KenoWinCalculator.h
#pragma once
#include <vector>
#include "RandomNumber.h"
class KenoWinCalculator
private:
std::vector<int> m_playerPick; // vector is used because the size is not fixed,
std::vector<int> m_ComputerPick; // player can choose any picks between 5 and 10 but computer's is fixed
int m_hits = 0;
public:
KenoWinCalculator(const std::vector<int> &playerPick);
~KenoWinCalculator() = default;
void generateComputerPick(RandomNumber* NumberGenerator); // Generate COMPUTER_PICK_COUNT unique computer picked random values
void calculateHits(); // Calculates no of common nos between player and computer pick
int getHitValue() const return m_hits;
int getPlayerPickCount() const return static_cast<int>(m_playerPick.size());
const std::vector<int>& getPlayerPickValues() const return m_playerPick;
std::vector<int> getComputerPick() const return m_ComputerPick;
;
KenoWinCalculator.cpp
#include <algorithm>
#include "kenoGameConstants.h"
#include "RandomNumber.h"
#include "KenoWinCalculator.h"
KenoWinCalculator::KenoWinCalculator(const std::vector<int> &playerPick)
m_playerPick = playerPick;
void KenoWinCalculator::generateComputerPick(RandomNumber* NumberGenerator) // Generate 10 unique computer picked random values
m_ComputerPick = NumberGenerator->GenerateAndGetVectorOfUniqueRandomNumbers(COMPUTER_PICK_COUNT);
void KenoWinCalculator::calculateHits()
for each (auto playerPick in m_playerPick)
if (std::find(m_ComputerPick.begin(), m_ComputerPick.end(), playerPick) != m_ComputerPick.end())
m_hits++;
c++ c++11 game
Earlier this week I provided this answer to the Original Poster of the question. Since I've already shown that I am not a C++ Guru, I have made the changes I suggested and I'm posting them for review. My question is what alternative Standard Template Library constructs I could have used to implement this. In case you aren't aware, copying is a major form of flattery.
The Basics of the game are:
The user creates a balance of credits to play the game.
Per game round the user enters a bet.
The user then has the choice of either picking between 5 and 10
numbers between 1 and 60 or allowing the computer to pick between 5
and 10 numbers between 1 and 60.Each game round is scored based on the number of picks and the number
of matches of those picks. Winnings are applied to the balance, bets
reduce the balance.A zero balance prevents game play and allows the user to add credits.
The goals I had when writing this alternate solution are:
Break the one file solution into multiple header and source files to
provide better object oriented programming and reuse.Replace all C programming language constructs with C++ programming.
Improve the readability and maintainability of the code.
Follow the Single Responsibility Principle.
Follow the DRY programming Principle.
Follow the KISS principle.
Reduce the number of lines of code in the solution.
Note: I was not successful in reducing the total number of lines of code, at least partially because breaking the code into header and source files adds some overhead lines of code, and because adding functions adds at least 3 lines of code for every new function. The original solutions is approximately 375 lines of code this alternate solution is approximately 440 lines of code.
One of the few major changes to the code, one is that the function kenoGameplay(Bank &credits)
was turned into a class. The other is that an std::map()
of multiple maps was used to replace a rather large case statement with embedded if statements in Award::awardWon(Bank &bank)
. A third was to decouple the Award
class from the Bank
class by adding another interface to the Bank
class. A fourth was to change a C style array of integers into a std::vector
. The final major change is that the random number generator now returns vectors of unique random numbers rather than a single random number and the interface is no longer a static member.
Possible Bugs in the Original that are Repeated Here
- If this were truly a Keno Game rather than a Lottery Game, then the
bet should be used as a multiplier of the award, there is currently
no reason to bet anything other than 1. - Rather than forcing the user to bet every time allow them to choose
to keep the same bet or change it. - This program still uses the C standard library
rand()
function
rather than more modern random number generators. This is offset
somewhat by the fact that the implementation is now hidden and
changing this will only force relinking after RandomNumber.cpp is
recompiled.
Because I used Visual Studio 2015, this alternate solution is not portable without edits to remove stdafx.h
from source files and replace #pragma once
with guard bandes in the header files.
A specific question about the code is there a single STL construct I can use to replace this function, I don't think std::equal()
will work since one vector is always length 15 and the other will vary between length 5 and length 10.
void KenoWinCalculator::calculateHits()
for each (auto playerPick in m_playerPick)
if (std::find(m_ComputerPick.begin(), m_ComputerPick.end(), playerPick) != m_ComputerPick.end())
m_hits++;
A second specific question I have is would it be better to use std::count()
over std::find()
in the above function.
EDIT
While I am specifically interested in STL improvements, all observations are good, since I'm far from perfect.
GetUserInput.h
#pragma once
#include <iostream>
#include <string>
template<typename T>
T PromptUserReturnResponse(const char* prompt)
T response;
std::cout << prompt << std::endl;
std::cin >> response;
std::cin.ignore(std::numeric_limits<std::streamsize>::max(), 'n');
return response;
kenoGameConstants.h
#pragma once
static const int MIN_PICK_VALUE = 1;
static const int MAX_PICK_VALUE = 60;
static const int COMPUTER_PICK_COUNT = 15;
KenoGame1.cpp
#include "stdafx.h"
#include <iostream>
#include <string>
#include "Bank.h"
#include "KenoGamePlay.h"
#include "GetUserInput.h"
std::string welcomeUser()
std::cout << "******** WELCOME TO WORLD OF KENO ************** n n";
std::string name = PromptUserReturnResponse<decltype(name)>("Please enter your name n");
std::cout << "Hello! " << name << "...nn" << std::endl;
return name;
Bank StartTheBank()
Bank credits;
credits.setBankBalance(PromptUserReturnResponse<int>("Please insert money"));
return credits;
int main()
int programStatus = EXIT_SUCCESS;
std::string name = welcomeUser();
kenoGamePlay kenoGameController(StartTheBank());
programStatus = kenoGameController.GameLoop();
return programStatus;
kenogameplay.h
#pragma once
#include <iostream>
#include <vector>
#include "Bank.h"
//#include "RandomNumber.h"
class kenoGamePlay
private:
Bank &m_credits;
class RandomNumber *m_NumberGenerator;
protected:
int PlayOneGame();
std::vector<int> QuickPickNumbers();
std::vector<int> PlayerPicksNumbers();
bool IsQuickPick();
void ValidateNumberAndAddOrReport(int num, std::vector<int> &gameNumbersToPlay);
void DisplayIntegersInVector(std::vector<int> numbersToPrint) const
for each (auto num in numbersToPrint)
std::cout << num << " ";
std::cout << "n";
public:
kenoGamePlay(Bank &credits);
~kenoGamePlay() = default;
int GameLoop();
;
kenogameplay.cpp
#include "stdafx.h"
#include <iostream>
#include <vector>
#include <string>
#include <algorithm>
#include "kenoGameConstants.h"
#include "kenogameplay.h"
#include "KenoWinCalculator.h"
#include "Award.h"
#include "GetUserInput.h"
#include "RandomNumber.h"
static const size_t QUIT_VALUE = 100;
static const size_t MAX_NOS_PICKED = 10;
static const size_t MIN_NOS_PICKED = 5;
static const size_t INSERT_MONEY = 1;
kenoGamePlay::kenoGamePlay(Bank &credits)
: m_creditscredits
m_NumberGenerator = new RandomNumber();
std::vector<int> kenoGamePlay::QuickPickNumbers()
quickPickCount > MAX_NOS_PICKED);
return m_NumberGenerator->GenerateAndGetVectorOfUniqueRandomNumbers(quickPickCount);
void kenoGamePlay::ValidateNumberAndAddOrReport(int num, std::vector<int> &gameNumbersToPlay)
if (num >= MIN_PICK_VALUE && num <= MAX_PICK_VALUE)
if (std::count(gameNumbersToPlay.begin(), gameNumbersToPlay.end(), num) != 0)
std::cout << "You already picked " << num << ". Please select another number. n";
else
gameNumbersToPlay.push_back(num);
else
std::cout << "You entered an invalid number. Please try again n";
std::vector<int> kenoGamePlay::PlayerPicksNumbers()
std::cout << "Pick between " << MIN_NOS_PICKED << " and " << MAX_NOS_PICKED <<
" unique numbers between " << MIN_PICK_VALUE << " and " << MAX_PICK_VALUE <<
". Enter " << QUIT_VALUE << " to QUIT entering numbersn";
int num = 0;
std::vector<int> gameNumbersToPlay;
while (gameNumbersToPlay.size() < MAX_NOS_PICKED)
num = PromptUserReturnResponse<decltype(num)>("Enter a number>>");
if (num == QUIT_VALUE)
if (gameNumbersToPlay.size() >= MIN_NOS_PICKED)
return gameNumbersToPlay;
else
std::cout << "You haven't selected enough numbers yet, please pick more numbers.n";
else
ValidateNumberAndAddOrReport(num, gameNumbersToPlay);
return gameNumbersToPlay;
int kenoGamePlay::GameLoop()
int programStatus = EXIT_SUCCESS;
char continueGameplay = 'y';
bool isBetInvalid = true;
int insertMoneyReduceBet = 0;
while (programStatus == EXIT_SUCCESS && continueGameplay != 'n')
if (m_credits.isEnoughBalanceToPlayGame()
return programStatus;
int kenoGamePlay::PlayOneGame()
int programStatus = EXIT_SUCCESS;
std::vector<int> nosPicked = (IsQuickPick()) ? QuickPickNumbers() : PlayerPicksNumbers();
KenoWinCalculator winCalc(nosPicked);
winCalc.generateComputerPick(m_NumberGenerator);
winCalc.calculateHits();
Award wonAward(winCalc);
wonAward.awardWon(m_credits);
std::vector<int> playerPickedValues = winCalc.getPlayerPickValues();
std::cout << "Your selection: t";
DisplayIntegersInVector(playerPickedValues);
std::cout << "nComputer selection: t";
DisplayIntegersInVector(winCalc.getComputerPick());
std::cout << "nno of hits: " << winCalc.getHitValue() << "n" << "Your bank balance is: $" << m_credits.getBankBalance() << "n";
std::cout.flush();
return programStatus;
bool kenoGamePlay::IsQuickPick()
return (PromptUserReturnResponse<char>("Do you want the computer to pick the numbers for you? [y]es or [n]o") == 'y');
Bank.h
#pragma once
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
~Bank() = default;
void setBet(int betValue) m_bet = betValue;
void setBankBalance(int balance) m_bankBalance += balance;
void updatebankBalance() m_bankBalance -= m_bet;
void depositWinnings(int winnings) m_bankBalance += winnings;
int getBankBalance() const return m_bankBalance;
bool isEnoughBalanceToPlayGame() const return m_bankBalance >= m_bet;
bool isBetValid() const return m_bet <= m_bankBalance && m_bet > 0;
;
RandomNumber.h
#pragma once
#include <vector>
class RandomNumber // Random number generator
private:
std::vector<int> UniqueRandomNumbersContainer;
protected:
int randomNumberGenerator();
public:
RandomNumber();
~RandomNumber();
void resetUniqueRandomNumbersContainer()
if (!UniqueRandomNumbersContainer.empty())
UniqueRandomNumbersContainer.clear();
std::vector<int> getaVectorOfUniqueRandomNumbers() const return UniqueRandomNumbersContainer;
std::vector<int> GenerateAndGetVectorOfUniqueRandomNumbers(size_t UniqueRandomNumberCount)
generateUniqueRandomNumbers(UniqueRandomNumberCount);
return UniqueRandomNumbersContainer;
void generateUniqueRandomNumbers(size_t UniqueRandomNumberCount);
;
RandomNumber.cpp
#include <ctime>
#include <algorithm>
#include "stdafx.h"
#include "RandomNumber.h"
#include "kenoGameConstants.h"
RandomNumber::RandomNumber()
srand(static_cast<unsigned int>(time(0)));
RandomNumber::~RandomNumber()
resetUniqueRandomNumbersContainer();
int RandomNumber::randomNumberGenerator()
int randomRange = MIN_PICK_VALUE + MAX_PICK_VALUE;
return (rand() % randomRange);
void RandomNumber::generateUniqueRandomNumbers(size_t UniqueRandomNumberCount)
if (!UniqueRandomNumbersContainer.empty())
UniqueRandomNumbersContainer.clear();
size_t generatedCount = 0;
while (generatedCount < UniqueRandomNumberCount)
int rngValue = randomNumberGenerator();
if (std::find(UniqueRandomNumbersContainer.begin(), UniqueRandomNumbersContainer.end(), rngValue) == UniqueRandomNumbersContainer.end())
UniqueRandomNumbersContainer.push_back(rngValue);
generatedCount++;
award.h
#pragma once
#include "KenoWinCalculator.h"
#include "Bank.h"
#include <map>
class Award
private: // This class maintains pay table
typedef std::map<int, int> HitMap;
typedef std::map<int, HitMap> PlayerPickMap;
int m_picks;
int m_hits;
PlayerPickMap MapSelector;
public:
Award(const KenoWinCalculator &calc);
~Award();
void awardWon(Bank &bank);
;
Award.cpp
#include <iostream>
#include "award.h"
#include "KenoWinCalculator.h"
Award::Award(const KenoWinCalculator &calc)
m_picks = calc.getPlayerPickCount();
m_hits = calc.getHitValue();
// In each of the follow tuples the first number is the number of hits or matches
// and the second number is the dollar reward. ie 4, 50 means 4 hits and $50.
HitMap Pick5Hits = 4, 50, 5, 100 ;
HitMap Pick6Hits = 4, 100, 5, 120, 6, 150 ;
HitMap Pick7Hits = 3, 80, 4, 90, 5, 100, 6, 120, 7, 150 ;
HitMap Pick8Hits = 3, 80, 4, 90, 5, 100, 6, 120, 7, 150, 8, 175 ;
HitMap Pick9Hits = 3, 80, 4, 90, 5, 100, 6, 120, 7, 150, 8, 175, 9, 200 ;
HitMap Pick10Hits = 3, 80, 4, 90, 5, 100, 6, 120, 7, 150, 8, 175, 9, 200, 10, 250 ;
MapSelector = 5, Pick5Hits, 6, Pick6Hits, 7, Pick7Hits, 8, Pick8Hits, 9, Pick9Hits, 10, Pick10Hits ;
Award::~Award()
for each(auto hitmap in MapSelector)
hitmap.second.clear();
MapSelector.clear();
void Award::awardWon(Bank &bank)
int amtWon = 0;
auto correctHitMap = MapSelector.find(m_picks);
if (correctHitMap != MapSelector.end())
HitMap HitMapTosearch = correctHitMap->second;
auto HitValueMap = HitMapTosearch.find(m_hits);
if (HitValueMap != HitMapTosearch.end())
amtWon = HitValueMap->second;
if (amtWon > 0)
std::cout << "Congratulations you won $" << amtWon << "!n";
bank.depositWinnings(amtWon);
else
std::cout << "Sorry you lostn";
std::cout << std::endl; // flush the output
KenoWinCalculator.h
#pragma once
#include <vector>
#include "RandomNumber.h"
class KenoWinCalculator
private:
std::vector<int> m_playerPick; // vector is used because the size is not fixed,
std::vector<int> m_ComputerPick; // player can choose any picks between 5 and 10 but computer's is fixed
int m_hits = 0;
public:
KenoWinCalculator(const std::vector<int> &playerPick);
~KenoWinCalculator() = default;
void generateComputerPick(RandomNumber* NumberGenerator); // Generate COMPUTER_PICK_COUNT unique computer picked random values
void calculateHits(); // Calculates no of common nos between player and computer pick
int getHitValue() const return m_hits;
int getPlayerPickCount() const return static_cast<int>(m_playerPick.size());
const std::vector<int>& getPlayerPickValues() const return m_playerPick;
std::vector<int> getComputerPick() const return m_ComputerPick;
;
KenoWinCalculator.cpp
#include <algorithm>
#include "kenoGameConstants.h"
#include "RandomNumber.h"
#include "KenoWinCalculator.h"
KenoWinCalculator::KenoWinCalculator(const std::vector<int> &playerPick)
m_playerPick = playerPick;
void KenoWinCalculator::generateComputerPick(RandomNumber* NumberGenerator) // Generate 10 unique computer picked random values
m_ComputerPick = NumberGenerator->GenerateAndGetVectorOfUniqueRandomNumbers(COMPUTER_PICK_COUNT);
void KenoWinCalculator::calculateHits()
for each (auto playerPick in m_playerPick)
if (std::find(m_ComputerPick.begin(), m_ComputerPick.end(), playerPick) != m_ComputerPick.end())
m_hits++;
c++ c++11 game
edited Jan 5 at 20:21
asked Jan 5 at 18:31
pacmaninbw
4,99321436
4,99321436
Just to make sure, are you only interested in improvements regarding STL usage or would you also like general hints and corrections?
â Ben Steffan
Jan 5 at 20:08
@BenSteffan General hints and suggestions are good, but I am interested in the STL usage. I'm aware that I could improve my naming conventions, the use of Camel Case versus First Cap is inconsistent. Feel free to review everything since it is a learning experience no matter what.
â pacmaninbw
Jan 5 at 20:14
1
Fine, then I'll go ahead.
â Ben Steffan
Jan 5 at 20:15
Does your program actually compile? A conforming compiler should reject it.
â Ben Steffan
Jan 5 at 21:14
1
I think I remember that Visual Studio allowed that one to slip. Give me 5 minutes, I'm nearly done with my answer that includes a description of what I am talking about.
â Ben Steffan
Jan 5 at 21:32
 |Â
show 1 more comment
Just to make sure, are you only interested in improvements regarding STL usage or would you also like general hints and corrections?
â Ben Steffan
Jan 5 at 20:08
@BenSteffan General hints and suggestions are good, but I am interested in the STL usage. I'm aware that I could improve my naming conventions, the use of Camel Case versus First Cap is inconsistent. Feel free to review everything since it is a learning experience no matter what.
â pacmaninbw
Jan 5 at 20:14
1
Fine, then I'll go ahead.
â Ben Steffan
Jan 5 at 20:15
Does your program actually compile? A conforming compiler should reject it.
â Ben Steffan
Jan 5 at 21:14
1
I think I remember that Visual Studio allowed that one to slip. Give me 5 minutes, I'm nearly done with my answer that includes a description of what I am talking about.
â Ben Steffan
Jan 5 at 21:32
Just to make sure, are you only interested in improvements regarding STL usage or would you also like general hints and corrections?
â Ben Steffan
Jan 5 at 20:08
Just to make sure, are you only interested in improvements regarding STL usage or would you also like general hints and corrections?
â Ben Steffan
Jan 5 at 20:08
@BenSteffan General hints and suggestions are good, but I am interested in the STL usage. I'm aware that I could improve my naming conventions, the use of Camel Case versus First Cap is inconsistent. Feel free to review everything since it is a learning experience no matter what.
â pacmaninbw
Jan 5 at 20:14
@BenSteffan General hints and suggestions are good, but I am interested in the STL usage. I'm aware that I could improve my naming conventions, the use of Camel Case versus First Cap is inconsistent. Feel free to review everything since it is a learning experience no matter what.
â pacmaninbw
Jan 5 at 20:14
1
1
Fine, then I'll go ahead.
â Ben Steffan
Jan 5 at 20:15
Fine, then I'll go ahead.
â Ben Steffan
Jan 5 at 20:15
Does your program actually compile? A conforming compiler should reject it.
â Ben Steffan
Jan 5 at 21:14
Does your program actually compile? A conforming compiler should reject it.
â Ben Steffan
Jan 5 at 21:14
1
1
I think I remember that Visual Studio allowed that one to slip. Give me 5 minutes, I'm nearly done with my answer that includes a description of what I am talking about.
â Ben Steffan
Jan 5 at 21:32
I think I remember that Visual Studio allowed that one to slip. Give me 5 minutes, I'm nearly done with my answer that includes a description of what I am talking about.
â Ben Steffan
Jan 5 at 21:32
 |Â
show 1 more comment
1 Answer
1
active
oldest
votes
up vote
1
down vote
accepted
A Serious Bug
I am sorry to be the bearer of bad message, but your whole programs behavior is currently undefined. You have stepped into the C++ reference-lifetime-trap. Lets take a look at this piece of code:
class kenoGamePlay
private:
Bank &m_credits;
//...
;
m_credits
is a reference to a Bank
-object. References as class members are rare and very dangerous, so let's pull out our alarm lights and go investigate whether the reference is justified and used correctly:
int main()
//...
kenoGamePlay kenoGameController(StartTheBank());
//...
Bingo. You're assigning a non-const, non-rvalue reference from a temporary, which is actually not allowed by the standard (and my compiler rejects it correctly). However, even if it were, you'd have a problem anyways: Since the lifetime of the temporary ends immediately, you now have a dangling reference that points to an invalid location in memory (= a location where no object of type Bank
lives). Doing accesses through such a reference is undefined behavior, and thus your whole program becomes undefined.
Fixing this, however, is not at all difficult: Just remove the reference and let m_credits
be a normal object of type Bank
.
In general, avoid using references as members unless you are 100% percent sure of what you're doing. In almost all cases, you are running a high risk of shooting yourself in the foot and blowing your whole leg off in the process.
Regarding KenoWinCalculator::calculateHits()
As far as my knowledge of the STL goes, there is no single function that would do what you require here. However, I came up with an implementation using both std::count_if
as well as std::find
, which, in my opinion, is about as concise as it will get:
void KenoWinCalculator::calculateHits()
m_hits += std::count_if(m_playerPick.begin(), m_playerPick.end(), [this] (int a)
return std::find(m_ComputerPick.begin(), m_ComputerPick.end(), a) != m_ComputerPick.end();
);
About STL Usage
if (std::count(gameNumbersToPlay.begin(), gameNumbersToPlay.end(), num) != 0)
: Why are you usingstd::count
here? You only care if an element exists or not, so you can terminate your search as soon as you have found a matching element, which is exactly howstd::find
behaves.Depending on how much you love the STL, you could consider replacing most of your
for
-loops withstd::for_each
. However, this is a disputable refactoring; many people are going to disagree with that style. Ultimately, it's a matter of personal taste.
Various Hints and Tips
for each (... in ...)
is not valid C++. In fact, when first reading your code, I was really taken aback because I thought that this shouldn't ever compile. However, it turns out (after researching) that this is actually a MSVC extension, albeit a really useless and horrible one, in my opinion. Please, please, please use the standardfor (... : ...)
syntax, lest you want a mob of C++ programmers with pitchforks and and burning torches show up at your door.Don't use
std::endl
. It's bad, because a)'n'
does the job just fine, and b)std::endl
actually also callsflush()
on your output stream, which is something you very rarely want. Just stick with'n'
and callflush()
directly when you mean to.Order your includes. Why? Because ordering includes makes it easier to verify whether all necessary includes are actually there, in two different ways: One, it makes it easier to manually verify that all necessary headers are there and two, the compiler will complain if you are missing an include in a header file if that header file is included before its missing header is included
(For example, imagine
A.h
depends on includingvector
. However, the author ofA.h
forgot the include, and now you're using the header in your project fileB.cpp
, which also requires#include <vector>
. If you first includeA.h
and thenvector
, your compiler will complain that it can't resolvestd::vector
inA.h
. If you includevector
first, however, this error does not surface because now the include is there. This is known as include shadowing.)So how should you be ordering those includes? First of all, nearly all people agree on alphabetic ordering. Secondly, it makes sense to group headers into categories such as "files from the current project", "library headers", "STL headers" etc. If you do this, it is recommended to write, from top to bottom, first the header the current
.cpp
file is implementing (if any), then headers from the current project (i.e. local/internal headers), then headers from different projects and libraries (i.e. external headers) and finally standard headers. This is most likely to prevent include shadowing bugs due to each header being checked in its implementation file (given that such a file exists).Always use
std::size_t
oversize_t
; the existence ofsize_t
in the global namespace is not guaranteed by the standard, but in thestd
namespace it is.Avoid including
iostream
everywhere. The reason for this is thatiostream
usually injects static instances forcout
,cerr
andcin
which can lead to binary bloat. More importantly, however,iostream
is a code smell, because usingstd::cout
and family is constraining your users and yourself. Imagine, for example, a bot, which wants to run some 1000 or 10000 games of Keno and log the results to a file, or a websocket, or any other I/O device. Currently, the only way for that bot to achieve this is by redirecting standard output, but that might not always be possible. Also, what if the bot actually only wants to log the output ofDisplayIntegersInVector()
to its destination, but doesn't care for the rest you write? Currently, it would need to parse the standard output for that, which, again, might be constraining or unfeasible.But if
iostream
is bad, how do you implement output functions? The answer is by refactoring your output functions to take anstd::ostream
as parameter and write to that instead. Syntax-wise, there is no change at all; the offered degree of flexibility, however, is much, much higher. (Don't forget to change#include <iostream>
to#include <ostream>
, though).kenoGamePlay::DisplayIntegersInVector
has two issues:- Why is that function part of
kenoGamePlay
? You don't do anything related to the class at all; this function should be freestanding. - The parameter
numbersToPrint
should not be passed by value, but byconst&
instead.std::vector
is expensive to copy, and the function does nothing at all that would justify this extra cost.
- Why is that function part of
Why are most member functions of
kenoGamePlay
protected? Is this class intended to be derived from? Doesn't look like it very much, since you have no virtual methods (not even a virtual destructor). If you still intend to derive from this, you should rethink your interface, if not, those members should be private.Prefer
using
overtypedef
. It's more readable and more flexible.Don't use single-letter string literals when not absolutely required. Writing
"n"
hurts performance, especially when'n'
does the same thing in the same amount of characters.You don't need to
default
all destructors manually; the compiler will generate a destructor for you automatically.
The bug I've found is somewhat unfortunate, but easy to fix. Apart from that, there are a lot of little things that you could do to improve your code, but apart from point 1 I didn't spot anything that would require immediate changes. However, it would be nice if you'd strive to make this code portable, i.e. remove #include "stdafx.h"
. Also, there are some irregularities in how you name things; you should decide on a coherent style for the sake of readability.
Regarding STL usage, I didn't actually find much that could be improved. Apart from replacing legacy C functionality (i.e. rand()
and time()
), there is not much to do in regards to making your code more C++-y.
Overall, I enjoyed reading your code. Keep up the good work!
1
Thanks, and LOL! FYI, the reference is my fault because I didn't catch it in the OP's question.
â pacmaninbw
Jan 5 at 22:14
@pacmaninbw You're welcome! I'm still very much astonished that MSVC doesn't reject that code.
â Ben Steffan
Jan 5 at 22:21
They must have been trying to placate PHP programmers that were forced to switch to C++. Note to self, copy code to Linux and compile with gcc or g++ before posting question.
â pacmaninbw
Jan 5 at 22:24
@BenSteffan, I had something similar but I didn't get even a warning on clang. When I approach things like this, I always ban constructors takingtype&&
andconst type&&
. There are better techniques described in this paper.
â Incomputable
Jan 6 at 11:57
add a comment |Â
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
1
down vote
accepted
A Serious Bug
I am sorry to be the bearer of bad message, but your whole programs behavior is currently undefined. You have stepped into the C++ reference-lifetime-trap. Lets take a look at this piece of code:
class kenoGamePlay
private:
Bank &m_credits;
//...
;
m_credits
is a reference to a Bank
-object. References as class members are rare and very dangerous, so let's pull out our alarm lights and go investigate whether the reference is justified and used correctly:
int main()
//...
kenoGamePlay kenoGameController(StartTheBank());
//...
Bingo. You're assigning a non-const, non-rvalue reference from a temporary, which is actually not allowed by the standard (and my compiler rejects it correctly). However, even if it were, you'd have a problem anyways: Since the lifetime of the temporary ends immediately, you now have a dangling reference that points to an invalid location in memory (= a location where no object of type Bank
lives). Doing accesses through such a reference is undefined behavior, and thus your whole program becomes undefined.
Fixing this, however, is not at all difficult: Just remove the reference and let m_credits
be a normal object of type Bank
.
In general, avoid using references as members unless you are 100% percent sure of what you're doing. In almost all cases, you are running a high risk of shooting yourself in the foot and blowing your whole leg off in the process.
Regarding KenoWinCalculator::calculateHits()
As far as my knowledge of the STL goes, there is no single function that would do what you require here. However, I came up with an implementation using both std::count_if
as well as std::find
, which, in my opinion, is about as concise as it will get:
void KenoWinCalculator::calculateHits()
m_hits += std::count_if(m_playerPick.begin(), m_playerPick.end(), [this] (int a)
return std::find(m_ComputerPick.begin(), m_ComputerPick.end(), a) != m_ComputerPick.end();
);
About STL Usage
if (std::count(gameNumbersToPlay.begin(), gameNumbersToPlay.end(), num) != 0)
: Why are you usingstd::count
here? You only care if an element exists or not, so you can terminate your search as soon as you have found a matching element, which is exactly howstd::find
behaves.Depending on how much you love the STL, you could consider replacing most of your
for
-loops withstd::for_each
. However, this is a disputable refactoring; many people are going to disagree with that style. Ultimately, it's a matter of personal taste.
Various Hints and Tips
for each (... in ...)
is not valid C++. In fact, when first reading your code, I was really taken aback because I thought that this shouldn't ever compile. However, it turns out (after researching) that this is actually a MSVC extension, albeit a really useless and horrible one, in my opinion. Please, please, please use the standardfor (... : ...)
syntax, lest you want a mob of C++ programmers with pitchforks and and burning torches show up at your door.Don't use
std::endl
. It's bad, because a)'n'
does the job just fine, and b)std::endl
actually also callsflush()
on your output stream, which is something you very rarely want. Just stick with'n'
and callflush()
directly when you mean to.Order your includes. Why? Because ordering includes makes it easier to verify whether all necessary includes are actually there, in two different ways: One, it makes it easier to manually verify that all necessary headers are there and two, the compiler will complain if you are missing an include in a header file if that header file is included before its missing header is included
(For example, imagine
A.h
depends on includingvector
. However, the author ofA.h
forgot the include, and now you're using the header in your project fileB.cpp
, which also requires#include <vector>
. If you first includeA.h
and thenvector
, your compiler will complain that it can't resolvestd::vector
inA.h
. If you includevector
first, however, this error does not surface because now the include is there. This is known as include shadowing.)So how should you be ordering those includes? First of all, nearly all people agree on alphabetic ordering. Secondly, it makes sense to group headers into categories such as "files from the current project", "library headers", "STL headers" etc. If you do this, it is recommended to write, from top to bottom, first the header the current
.cpp
file is implementing (if any), then headers from the current project (i.e. local/internal headers), then headers from different projects and libraries (i.e. external headers) and finally standard headers. This is most likely to prevent include shadowing bugs due to each header being checked in its implementation file (given that such a file exists).Always use
std::size_t
oversize_t
; the existence ofsize_t
in the global namespace is not guaranteed by the standard, but in thestd
namespace it is.Avoid including
iostream
everywhere. The reason for this is thatiostream
usually injects static instances forcout
,cerr
andcin
which can lead to binary bloat. More importantly, however,iostream
is a code smell, because usingstd::cout
and family is constraining your users and yourself. Imagine, for example, a bot, which wants to run some 1000 or 10000 games of Keno and log the results to a file, or a websocket, or any other I/O device. Currently, the only way for that bot to achieve this is by redirecting standard output, but that might not always be possible. Also, what if the bot actually only wants to log the output ofDisplayIntegersInVector()
to its destination, but doesn't care for the rest you write? Currently, it would need to parse the standard output for that, which, again, might be constraining or unfeasible.But if
iostream
is bad, how do you implement output functions? The answer is by refactoring your output functions to take anstd::ostream
as parameter and write to that instead. Syntax-wise, there is no change at all; the offered degree of flexibility, however, is much, much higher. (Don't forget to change#include <iostream>
to#include <ostream>
, though).kenoGamePlay::DisplayIntegersInVector
has two issues:- Why is that function part of
kenoGamePlay
? You don't do anything related to the class at all; this function should be freestanding. - The parameter
numbersToPrint
should not be passed by value, but byconst&
instead.std::vector
is expensive to copy, and the function does nothing at all that would justify this extra cost.
- Why is that function part of
Why are most member functions of
kenoGamePlay
protected? Is this class intended to be derived from? Doesn't look like it very much, since you have no virtual methods (not even a virtual destructor). If you still intend to derive from this, you should rethink your interface, if not, those members should be private.Prefer
using
overtypedef
. It's more readable and more flexible.Don't use single-letter string literals when not absolutely required. Writing
"n"
hurts performance, especially when'n'
does the same thing in the same amount of characters.You don't need to
default
all destructors manually; the compiler will generate a destructor for you automatically.
The bug I've found is somewhat unfortunate, but easy to fix. Apart from that, there are a lot of little things that you could do to improve your code, but apart from point 1 I didn't spot anything that would require immediate changes. However, it would be nice if you'd strive to make this code portable, i.e. remove #include "stdafx.h"
. Also, there are some irregularities in how you name things; you should decide on a coherent style for the sake of readability.
Regarding STL usage, I didn't actually find much that could be improved. Apart from replacing legacy C functionality (i.e. rand()
and time()
), there is not much to do in regards to making your code more C++-y.
Overall, I enjoyed reading your code. Keep up the good work!
1
Thanks, and LOL! FYI, the reference is my fault because I didn't catch it in the OP's question.
â pacmaninbw
Jan 5 at 22:14
@pacmaninbw You're welcome! I'm still very much astonished that MSVC doesn't reject that code.
â Ben Steffan
Jan 5 at 22:21
They must have been trying to placate PHP programmers that were forced to switch to C++. Note to self, copy code to Linux and compile with gcc or g++ before posting question.
â pacmaninbw
Jan 5 at 22:24
@BenSteffan, I had something similar but I didn't get even a warning on clang. When I approach things like this, I always ban constructors takingtype&&
andconst type&&
. There are better techniques described in this paper.
â Incomputable
Jan 6 at 11:57
add a comment |Â
up vote
1
down vote
accepted
A Serious Bug
I am sorry to be the bearer of bad message, but your whole programs behavior is currently undefined. You have stepped into the C++ reference-lifetime-trap. Lets take a look at this piece of code:
class kenoGamePlay
private:
Bank &m_credits;
//...
;
m_credits
is a reference to a Bank
-object. References as class members are rare and very dangerous, so let's pull out our alarm lights and go investigate whether the reference is justified and used correctly:
int main()
//...
kenoGamePlay kenoGameController(StartTheBank());
//...
Bingo. You're assigning a non-const, non-rvalue reference from a temporary, which is actually not allowed by the standard (and my compiler rejects it correctly). However, even if it were, you'd have a problem anyways: Since the lifetime of the temporary ends immediately, you now have a dangling reference that points to an invalid location in memory (= a location where no object of type Bank
lives). Doing accesses through such a reference is undefined behavior, and thus your whole program becomes undefined.
Fixing this, however, is not at all difficult: Just remove the reference and let m_credits
be a normal object of type Bank
.
In general, avoid using references as members unless you are 100% percent sure of what you're doing. In almost all cases, you are running a high risk of shooting yourself in the foot and blowing your whole leg off in the process.
Regarding KenoWinCalculator::calculateHits()
As far as my knowledge of the STL goes, there is no single function that would do what you require here. However, I came up with an implementation using both std::count_if
as well as std::find
, which, in my opinion, is about as concise as it will get:
void KenoWinCalculator::calculateHits()
m_hits += std::count_if(m_playerPick.begin(), m_playerPick.end(), [this] (int a)
return std::find(m_ComputerPick.begin(), m_ComputerPick.end(), a) != m_ComputerPick.end();
);
About STL Usage
if (std::count(gameNumbersToPlay.begin(), gameNumbersToPlay.end(), num) != 0)
: Why are you usingstd::count
here? You only care if an element exists or not, so you can terminate your search as soon as you have found a matching element, which is exactly howstd::find
behaves.Depending on how much you love the STL, you could consider replacing most of your
for
-loops withstd::for_each
. However, this is a disputable refactoring; many people are going to disagree with that style. Ultimately, it's a matter of personal taste.
Various Hints and Tips
for each (... in ...)
is not valid C++. In fact, when first reading your code, I was really taken aback because I thought that this shouldn't ever compile. However, it turns out (after researching) that this is actually a MSVC extension, albeit a really useless and horrible one, in my opinion. Please, please, please use the standardfor (... : ...)
syntax, lest you want a mob of C++ programmers with pitchforks and and burning torches show up at your door.Don't use
std::endl
. It's bad, because a)'n'
does the job just fine, and b)std::endl
actually also callsflush()
on your output stream, which is something you very rarely want. Just stick with'n'
and callflush()
directly when you mean to.Order your includes. Why? Because ordering includes makes it easier to verify whether all necessary includes are actually there, in two different ways: One, it makes it easier to manually verify that all necessary headers are there and two, the compiler will complain if you are missing an include in a header file if that header file is included before its missing header is included
(For example, imagine
A.h
depends on includingvector
. However, the author ofA.h
forgot the include, and now you're using the header in your project fileB.cpp
, which also requires#include <vector>
. If you first includeA.h
and thenvector
, your compiler will complain that it can't resolvestd::vector
inA.h
. If you includevector
first, however, this error does not surface because now the include is there. This is known as include shadowing.)So how should you be ordering those includes? First of all, nearly all people agree on alphabetic ordering. Secondly, it makes sense to group headers into categories such as "files from the current project", "library headers", "STL headers" etc. If you do this, it is recommended to write, from top to bottom, first the header the current
.cpp
file is implementing (if any), then headers from the current project (i.e. local/internal headers), then headers from different projects and libraries (i.e. external headers) and finally standard headers. This is most likely to prevent include shadowing bugs due to each header being checked in its implementation file (given that such a file exists).Always use
std::size_t
oversize_t
; the existence ofsize_t
in the global namespace is not guaranteed by the standard, but in thestd
namespace it is.Avoid including
iostream
everywhere. The reason for this is thatiostream
usually injects static instances forcout
,cerr
andcin
which can lead to binary bloat. More importantly, however,iostream
is a code smell, because usingstd::cout
and family is constraining your users and yourself. Imagine, for example, a bot, which wants to run some 1000 or 10000 games of Keno and log the results to a file, or a websocket, or any other I/O device. Currently, the only way for that bot to achieve this is by redirecting standard output, but that might not always be possible. Also, what if the bot actually only wants to log the output ofDisplayIntegersInVector()
to its destination, but doesn't care for the rest you write? Currently, it would need to parse the standard output for that, which, again, might be constraining or unfeasible.But if
iostream
is bad, how do you implement output functions? The answer is by refactoring your output functions to take anstd::ostream
as parameter and write to that instead. Syntax-wise, there is no change at all; the offered degree of flexibility, however, is much, much higher. (Don't forget to change#include <iostream>
to#include <ostream>
, though).kenoGamePlay::DisplayIntegersInVector
has two issues:- Why is that function part of
kenoGamePlay
? You don't do anything related to the class at all; this function should be freestanding. - The parameter
numbersToPrint
should not be passed by value, but byconst&
instead.std::vector
is expensive to copy, and the function does nothing at all that would justify this extra cost.
- Why is that function part of
Why are most member functions of
kenoGamePlay
protected? Is this class intended to be derived from? Doesn't look like it very much, since you have no virtual methods (not even a virtual destructor). If you still intend to derive from this, you should rethink your interface, if not, those members should be private.Prefer
using
overtypedef
. It's more readable and more flexible.Don't use single-letter string literals when not absolutely required. Writing
"n"
hurts performance, especially when'n'
does the same thing in the same amount of characters.You don't need to
default
all destructors manually; the compiler will generate a destructor for you automatically.
The bug I've found is somewhat unfortunate, but easy to fix. Apart from that, there are a lot of little things that you could do to improve your code, but apart from point 1 I didn't spot anything that would require immediate changes. However, it would be nice if you'd strive to make this code portable, i.e. remove #include "stdafx.h"
. Also, there are some irregularities in how you name things; you should decide on a coherent style for the sake of readability.
Regarding STL usage, I didn't actually find much that could be improved. Apart from replacing legacy C functionality (i.e. rand()
and time()
), there is not much to do in regards to making your code more C++-y.
Overall, I enjoyed reading your code. Keep up the good work!
1
Thanks, and LOL! FYI, the reference is my fault because I didn't catch it in the OP's question.
â pacmaninbw
Jan 5 at 22:14
@pacmaninbw You're welcome! I'm still very much astonished that MSVC doesn't reject that code.
â Ben Steffan
Jan 5 at 22:21
They must have been trying to placate PHP programmers that were forced to switch to C++. Note to self, copy code to Linux and compile with gcc or g++ before posting question.
â pacmaninbw
Jan 5 at 22:24
@BenSteffan, I had something similar but I didn't get even a warning on clang. When I approach things like this, I always ban constructors takingtype&&
andconst type&&
. There are better techniques described in this paper.
â Incomputable
Jan 6 at 11:57
add a comment |Â
up vote
1
down vote
accepted
up vote
1
down vote
accepted
A Serious Bug
I am sorry to be the bearer of bad message, but your whole programs behavior is currently undefined. You have stepped into the C++ reference-lifetime-trap. Lets take a look at this piece of code:
class kenoGamePlay
private:
Bank &m_credits;
//...
;
m_credits
is a reference to a Bank
-object. References as class members are rare and very dangerous, so let's pull out our alarm lights and go investigate whether the reference is justified and used correctly:
int main()
//...
kenoGamePlay kenoGameController(StartTheBank());
//...
Bingo. You're assigning a non-const, non-rvalue reference from a temporary, which is actually not allowed by the standard (and my compiler rejects it correctly). However, even if it were, you'd have a problem anyways: Since the lifetime of the temporary ends immediately, you now have a dangling reference that points to an invalid location in memory (= a location where no object of type Bank
lives). Doing accesses through such a reference is undefined behavior, and thus your whole program becomes undefined.
Fixing this, however, is not at all difficult: Just remove the reference and let m_credits
be a normal object of type Bank
.
In general, avoid using references as members unless you are 100% percent sure of what you're doing. In almost all cases, you are running a high risk of shooting yourself in the foot and blowing your whole leg off in the process.
Regarding KenoWinCalculator::calculateHits()
As far as my knowledge of the STL goes, there is no single function that would do what you require here. However, I came up with an implementation using both std::count_if
as well as std::find
, which, in my opinion, is about as concise as it will get:
void KenoWinCalculator::calculateHits()
m_hits += std::count_if(m_playerPick.begin(), m_playerPick.end(), [this] (int a)
return std::find(m_ComputerPick.begin(), m_ComputerPick.end(), a) != m_ComputerPick.end();
);
About STL Usage
if (std::count(gameNumbersToPlay.begin(), gameNumbersToPlay.end(), num) != 0)
: Why are you usingstd::count
here? You only care if an element exists or not, so you can terminate your search as soon as you have found a matching element, which is exactly howstd::find
behaves.Depending on how much you love the STL, you could consider replacing most of your
for
-loops withstd::for_each
. However, this is a disputable refactoring; many people are going to disagree with that style. Ultimately, it's a matter of personal taste.
Various Hints and Tips
for each (... in ...)
is not valid C++. In fact, when first reading your code, I was really taken aback because I thought that this shouldn't ever compile. However, it turns out (after researching) that this is actually a MSVC extension, albeit a really useless and horrible one, in my opinion. Please, please, please use the standardfor (... : ...)
syntax, lest you want a mob of C++ programmers with pitchforks and and burning torches show up at your door.Don't use
std::endl
. It's bad, because a)'n'
does the job just fine, and b)std::endl
actually also callsflush()
on your output stream, which is something you very rarely want. Just stick with'n'
and callflush()
directly when you mean to.Order your includes. Why? Because ordering includes makes it easier to verify whether all necessary includes are actually there, in two different ways: One, it makes it easier to manually verify that all necessary headers are there and two, the compiler will complain if you are missing an include in a header file if that header file is included before its missing header is included
(For example, imagine
A.h
depends on includingvector
. However, the author ofA.h
forgot the include, and now you're using the header in your project fileB.cpp
, which also requires#include <vector>
. If you first includeA.h
and thenvector
, your compiler will complain that it can't resolvestd::vector
inA.h
. If you includevector
first, however, this error does not surface because now the include is there. This is known as include shadowing.)So how should you be ordering those includes? First of all, nearly all people agree on alphabetic ordering. Secondly, it makes sense to group headers into categories such as "files from the current project", "library headers", "STL headers" etc. If you do this, it is recommended to write, from top to bottom, first the header the current
.cpp
file is implementing (if any), then headers from the current project (i.e. local/internal headers), then headers from different projects and libraries (i.e. external headers) and finally standard headers. This is most likely to prevent include shadowing bugs due to each header being checked in its implementation file (given that such a file exists).Always use
std::size_t
oversize_t
; the existence ofsize_t
in the global namespace is not guaranteed by the standard, but in thestd
namespace it is.Avoid including
iostream
everywhere. The reason for this is thatiostream
usually injects static instances forcout
,cerr
andcin
which can lead to binary bloat. More importantly, however,iostream
is a code smell, because usingstd::cout
and family is constraining your users and yourself. Imagine, for example, a bot, which wants to run some 1000 or 10000 games of Keno and log the results to a file, or a websocket, or any other I/O device. Currently, the only way for that bot to achieve this is by redirecting standard output, but that might not always be possible. Also, what if the bot actually only wants to log the output ofDisplayIntegersInVector()
to its destination, but doesn't care for the rest you write? Currently, it would need to parse the standard output for that, which, again, might be constraining or unfeasible.But if
iostream
is bad, how do you implement output functions? The answer is by refactoring your output functions to take anstd::ostream
as parameter and write to that instead. Syntax-wise, there is no change at all; the offered degree of flexibility, however, is much, much higher. (Don't forget to change#include <iostream>
to#include <ostream>
, though).kenoGamePlay::DisplayIntegersInVector
has two issues:- Why is that function part of
kenoGamePlay
? You don't do anything related to the class at all; this function should be freestanding. - The parameter
numbersToPrint
should not be passed by value, but byconst&
instead.std::vector
is expensive to copy, and the function does nothing at all that would justify this extra cost.
- Why is that function part of
Why are most member functions of
kenoGamePlay
protected? Is this class intended to be derived from? Doesn't look like it very much, since you have no virtual methods (not even a virtual destructor). If you still intend to derive from this, you should rethink your interface, if not, those members should be private.Prefer
using
overtypedef
. It's more readable and more flexible.Don't use single-letter string literals when not absolutely required. Writing
"n"
hurts performance, especially when'n'
does the same thing in the same amount of characters.You don't need to
default
all destructors manually; the compiler will generate a destructor for you automatically.
The bug I've found is somewhat unfortunate, but easy to fix. Apart from that, there are a lot of little things that you could do to improve your code, but apart from point 1 I didn't spot anything that would require immediate changes. However, it would be nice if you'd strive to make this code portable, i.e. remove #include "stdafx.h"
. Also, there are some irregularities in how you name things; you should decide on a coherent style for the sake of readability.
Regarding STL usage, I didn't actually find much that could be improved. Apart from replacing legacy C functionality (i.e. rand()
and time()
), there is not much to do in regards to making your code more C++-y.
Overall, I enjoyed reading your code. Keep up the good work!
A Serious Bug
I am sorry to be the bearer of bad message, but your whole programs behavior is currently undefined. You have stepped into the C++ reference-lifetime-trap. Lets take a look at this piece of code:
class kenoGamePlay
private:
Bank &m_credits;
//...
;
m_credits
is a reference to a Bank
-object. References as class members are rare and very dangerous, so let's pull out our alarm lights and go investigate whether the reference is justified and used correctly:
int main()
//...
kenoGamePlay kenoGameController(StartTheBank());
//...
Bingo. You're assigning a non-const, non-rvalue reference from a temporary, which is actually not allowed by the standard (and my compiler rejects it correctly). However, even if it were, you'd have a problem anyways: Since the lifetime of the temporary ends immediately, you now have a dangling reference that points to an invalid location in memory (= a location where no object of type Bank
lives). Doing accesses through such a reference is undefined behavior, and thus your whole program becomes undefined.
Fixing this, however, is not at all difficult: Just remove the reference and let m_credits
be a normal object of type Bank
.
In general, avoid using references as members unless you are 100% percent sure of what you're doing. In almost all cases, you are running a high risk of shooting yourself in the foot and blowing your whole leg off in the process.
Regarding KenoWinCalculator::calculateHits()
As far as my knowledge of the STL goes, there is no single function that would do what you require here. However, I came up with an implementation using both std::count_if
as well as std::find
, which, in my opinion, is about as concise as it will get:
void KenoWinCalculator::calculateHits()
m_hits += std::count_if(m_playerPick.begin(), m_playerPick.end(), [this] (int a)
return std::find(m_ComputerPick.begin(), m_ComputerPick.end(), a) != m_ComputerPick.end();
);
About STL Usage
if (std::count(gameNumbersToPlay.begin(), gameNumbersToPlay.end(), num) != 0)
: Why are you usingstd::count
here? You only care if an element exists or not, so you can terminate your search as soon as you have found a matching element, which is exactly howstd::find
behaves.Depending on how much you love the STL, you could consider replacing most of your
for
-loops withstd::for_each
. However, this is a disputable refactoring; many people are going to disagree with that style. Ultimately, it's a matter of personal taste.
Various Hints and Tips
for each (... in ...)
is not valid C++. In fact, when first reading your code, I was really taken aback because I thought that this shouldn't ever compile. However, it turns out (after researching) that this is actually a MSVC extension, albeit a really useless and horrible one, in my opinion. Please, please, please use the standardfor (... : ...)
syntax, lest you want a mob of C++ programmers with pitchforks and and burning torches show up at your door.Don't use
std::endl
. It's bad, because a)'n'
does the job just fine, and b)std::endl
actually also callsflush()
on your output stream, which is something you very rarely want. Just stick with'n'
and callflush()
directly when you mean to.Order your includes. Why? Because ordering includes makes it easier to verify whether all necessary includes are actually there, in two different ways: One, it makes it easier to manually verify that all necessary headers are there and two, the compiler will complain if you are missing an include in a header file if that header file is included before its missing header is included
(For example, imagine
A.h
depends on includingvector
. However, the author ofA.h
forgot the include, and now you're using the header in your project fileB.cpp
, which also requires#include <vector>
. If you first includeA.h
and thenvector
, your compiler will complain that it can't resolvestd::vector
inA.h
. If you includevector
first, however, this error does not surface because now the include is there. This is known as include shadowing.)So how should you be ordering those includes? First of all, nearly all people agree on alphabetic ordering. Secondly, it makes sense to group headers into categories such as "files from the current project", "library headers", "STL headers" etc. If you do this, it is recommended to write, from top to bottom, first the header the current
.cpp
file is implementing (if any), then headers from the current project (i.e. local/internal headers), then headers from different projects and libraries (i.e. external headers) and finally standard headers. This is most likely to prevent include shadowing bugs due to each header being checked in its implementation file (given that such a file exists).Always use
std::size_t
oversize_t
; the existence ofsize_t
in the global namespace is not guaranteed by the standard, but in thestd
namespace it is.Avoid including
iostream
everywhere. The reason for this is thatiostream
usually injects static instances forcout
,cerr
andcin
which can lead to binary bloat. More importantly, however,iostream
is a code smell, because usingstd::cout
and family is constraining your users and yourself. Imagine, for example, a bot, which wants to run some 1000 or 10000 games of Keno and log the results to a file, or a websocket, or any other I/O device. Currently, the only way for that bot to achieve this is by redirecting standard output, but that might not always be possible. Also, what if the bot actually only wants to log the output ofDisplayIntegersInVector()
to its destination, but doesn't care for the rest you write? Currently, it would need to parse the standard output for that, which, again, might be constraining or unfeasible.But if
iostream
is bad, how do you implement output functions? The answer is by refactoring your output functions to take anstd::ostream
as parameter and write to that instead. Syntax-wise, there is no change at all; the offered degree of flexibility, however, is much, much higher. (Don't forget to change#include <iostream>
to#include <ostream>
, though).kenoGamePlay::DisplayIntegersInVector
has two issues:- Why is that function part of
kenoGamePlay
? You don't do anything related to the class at all; this function should be freestanding. - The parameter
numbersToPrint
should not be passed by value, but byconst&
instead.std::vector
is expensive to copy, and the function does nothing at all that would justify this extra cost.
- Why is that function part of
Why are most member functions of
kenoGamePlay
protected? Is this class intended to be derived from? Doesn't look like it very much, since you have no virtual methods (not even a virtual destructor). If you still intend to derive from this, you should rethink your interface, if not, those members should be private.Prefer
using
overtypedef
. It's more readable and more flexible.Don't use single-letter string literals when not absolutely required. Writing
"n"
hurts performance, especially when'n'
does the same thing in the same amount of characters.You don't need to
default
all destructors manually; the compiler will generate a destructor for you automatically.
The bug I've found is somewhat unfortunate, but easy to fix. Apart from that, there are a lot of little things that you could do to improve your code, but apart from point 1 I didn't spot anything that would require immediate changes. However, it would be nice if you'd strive to make this code portable, i.e. remove #include "stdafx.h"
. Also, there are some irregularities in how you name things; you should decide on a coherent style for the sake of readability.
Regarding STL usage, I didn't actually find much that could be improved. Apart from replacing legacy C functionality (i.e. rand()
and time()
), there is not much to do in regards to making your code more C++-y.
Overall, I enjoyed reading your code. Keep up the good work!
answered Jan 5 at 21:45
Ben Steffan
4,85011234
4,85011234
1
Thanks, and LOL! FYI, the reference is my fault because I didn't catch it in the OP's question.
â pacmaninbw
Jan 5 at 22:14
@pacmaninbw You're welcome! I'm still very much astonished that MSVC doesn't reject that code.
â Ben Steffan
Jan 5 at 22:21
They must have been trying to placate PHP programmers that were forced to switch to C++. Note to self, copy code to Linux and compile with gcc or g++ before posting question.
â pacmaninbw
Jan 5 at 22:24
@BenSteffan, I had something similar but I didn't get even a warning on clang. When I approach things like this, I always ban constructors takingtype&&
andconst type&&
. There are better techniques described in this paper.
â Incomputable
Jan 6 at 11:57
add a comment |Â
1
Thanks, and LOL! FYI, the reference is my fault because I didn't catch it in the OP's question.
â pacmaninbw
Jan 5 at 22:14
@pacmaninbw You're welcome! I'm still very much astonished that MSVC doesn't reject that code.
â Ben Steffan
Jan 5 at 22:21
They must have been trying to placate PHP programmers that were forced to switch to C++. Note to self, copy code to Linux and compile with gcc or g++ before posting question.
â pacmaninbw
Jan 5 at 22:24
@BenSteffan, I had something similar but I didn't get even a warning on clang. When I approach things like this, I always ban constructors takingtype&&
andconst type&&
. There are better techniques described in this paper.
â Incomputable
Jan 6 at 11:57
1
1
Thanks, and LOL! FYI, the reference is my fault because I didn't catch it in the OP's question.
â pacmaninbw
Jan 5 at 22:14
Thanks, and LOL! FYI, the reference is my fault because I didn't catch it in the OP's question.
â pacmaninbw
Jan 5 at 22:14
@pacmaninbw You're welcome! I'm still very much astonished that MSVC doesn't reject that code.
â Ben Steffan
Jan 5 at 22:21
@pacmaninbw You're welcome! I'm still very much astonished that MSVC doesn't reject that code.
â Ben Steffan
Jan 5 at 22:21
They must have been trying to placate PHP programmers that were forced to switch to C++. Note to self, copy code to Linux and compile with gcc or g++ before posting question.
â pacmaninbw
Jan 5 at 22:24
They must have been trying to placate PHP programmers that were forced to switch to C++. Note to self, copy code to Linux and compile with gcc or g++ before posting question.
â pacmaninbw
Jan 5 at 22:24
@BenSteffan, I had something similar but I didn't get even a warning on clang. When I approach things like this, I always ban constructors taking
type&&
and const type&&
. There are better techniques described in this paper.â Incomputable
Jan 6 at 11:57
@BenSteffan, I had something similar but I didn't get even a warning on clang. When I approach things like this, I always ban constructors taking
type&&
and const type&&
. There are better techniques described in this paper.â Incomputable
Jan 6 at 11:57
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f184387%2fa-simple-console-based-keno-lottery-game-in-c%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Just to make sure, are you only interested in improvements regarding STL usage or would you also like general hints and corrections?
â Ben Steffan
Jan 5 at 20:08
@BenSteffan General hints and suggestions are good, but I am interested in the STL usage. I'm aware that I could improve my naming conventions, the use of Camel Case versus First Cap is inconsistent. Feel free to review everything since it is a learning experience no matter what.
â pacmaninbw
Jan 5 at 20:14
1
Fine, then I'll go ahead.
â Ben Steffan
Jan 5 at 20:15
Does your program actually compile? A conforming compiler should reject it.
â Ben Steffan
Jan 5 at 21:14
1
I think I remember that Visual Studio allowed that one to slip. Give me 5 minutes, I'm nearly done with my answer that includes a description of what I am talking about.
â Ben Steffan
Jan 5 at 21:32