EDIT
While I am specifically interested in STL improvements, all observations are good, since I'm far from perfect.
GetUserInput.h
Stack Exchange network consists of 183 Q&A communities including Stack Overflow, the largest, most trusted online community for developers to learn, share their knowledge, and build their careers.
Visit Stack ExchangeTeams
Learn more about TeamsEDIT
While I am specifically interested in STL improvements, all observations are good, since I'm far from perfect.
GetUserInput.h
GetUserInput.h
EDIT
While I am specifically interested in STL improvements, all observations are good, since I'm far from perfect.
GetUserInput.h
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
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.
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 << "...\n\n" << 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_credits{credits}
{
m_NumberGenerator = new RandomNumber();
}
std::vector<int> kenoGamePlay::QuickPickNumbers()
{
int quickPickCount = 0;
char* Prompt = "How many number do you want the computer to pick (5-10)?";
do {
quickPickCount = PromptUserReturnResponse<decltype(quickPickCount)>(Prompt);
Prompt = "Invalid count value. Choose between 5 and 10 only.?";
} while (quickPickCount < MIN_NOS_PICKED || 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 numbers\n";
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() || isBetInvalid == true)
{ // check if enough balance to play game
m_credits.setBet(PromptUserReturnResponse<int>("Place your bet"));
if (m_credits.isBetValid()) {
m_credits.updatebankBalance();
programStatus = PlayOneGame();
continueGameplay = PromptUserReturnResponse<decltype(continueGameplay)>("GAME OVER \nDo you want to play another round \n'y' to continue and 'n' to quit playing \n");
}
else
{
std::cout << "Invalid bet value." << std::endl;
isBetInvalid = true;
}
}
else
{
std::cout << "Not enough balance/valid bet. Please insert money( press 1) or place valid bet(press 2) to max bank balance " << m_credits.getBankBalance() << std::endl;
std::cin >> insertMoneyReduceBet;
if (insertMoneyReduceBet == INSERT_MONEY) {
insertMoneyReduceBet = PromptUserReturnResponse<decltype(insertMoneyReduceBet)>("Insert money ");
m_credits.setBankBalance(insertMoneyReduceBet);
}
else
isBetInvalid = true;
}
}
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 lost\n";
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++;
}
}
}