patterncppModerate
Rock-paper-scissors Game in C++
Viewed 0 times
scissorspapergamerock
Problem
Below you will find my first attempt at a C++ implementation of the classic game Rock-paper-scissors. If someone could look it over and point out or suggest any things that I could change to improve it, that would be brilliant and I would greatly appreciate it.
Player.h
Player.cpp
Main.cpp
```
#include "Player.cpp"
#include
#include
#include
#include
using namespace std;
void determineWinner(unique_ptr const & p1, unique_ptr const & p2)
{
if(p1->getChoice()=="rock" && p2->getChoice()=="scissors")
{
cout getName() getChoice()=="scissors" && p2->getChoice()=="rock")
{
cout getName() getChoice()=="paper" && p2->getChoice()=="rock")
{
cout getName() getChoice()=="rock" && p2->
Player.h
#ifndef PLAYER_H
#define PLAYER_H
#include
#include
#include
#include
class Player
{
private:
std::string name;
std::string choice;
public:
Player(std::string Name);
void setName(std::string Name);
std::string getName() const;
void setChoice(std::string c);
std::string makeChoice();
std::string getChoice() const;
};
#endifPlayer.cpp
#include "Player.h"
Player::Player(std::string Name)
{
name=Name;
choice=" ";
}
void Player::setName(std::string Name)
{
name=Name;
}
std::string Player::getName() const
{
return name;
}
std::string Player::makeChoice()
{
std::random_device rd;
std::mt19937 gen(rd());
std::uniform_int_distribution<> dis(1, 3);
int playerChoice=dis(gen);
switch(playerChoice)
{
case 1:
choice="rock";
break;
case 2:
choice="paper";
break;
case 3:
choice="scissors";
break;
default:
std::cout << "Please make a valid choice" << std::endl;
break;
}
return choice;
}
void Player::setChoice(std::string c)
{
choice=c;
}
std::string Player::getChoice() const
{
return choice;
}Main.cpp
```
#include "Player.cpp"
#include
#include
#include
#include
using namespace std;
void determineWinner(unique_ptr const & p1, unique_ptr const & p2)
{
if(p1->getChoice()=="rock" && p2->getChoice()=="scissors")
{
cout getName() getChoice()=="scissors" && p2->getChoice()=="rock")
{
cout getName() getChoice()=="paper" && p2->getChoice()=="rock")
{
cout getName() getChoice()=="rock" && p2->
Solution
You have all the game logic in a single function -
This section in particular is essentially duplicated. You can create a generic function like this:
If you really want to specify player turn, you could adjust it like this:
You could easily adjust this to be a string name, or any type you want that has an output operator.
One other thing I see is you always use
playRockPaperScissors2(). If you extracted the logic, you could easily reuse sections of it, perhaps for other games, perhaps to make a computer/human version.cout > player1Choice;
switch(player1Choice)
{
case 'r':
p1->setChoice("rock");
break;
case 'p':
p1->setChoice("paper");
break;
case 's':
p1->setChoice("scissors");
break;
default:
cout << "Please make a valid choice" << endl;
break;
}This section in particular is essentially duplicated. You can create a generic function like this:
std::string RPSPlayerChoice() {
cout > playerChoice;
do {
switch(playerChoice)
{
case 'r':
return "rock";
case 'p':
return "paper";
case 's':
return "scissors";
default:
cout << "Please make a valid choice" << endl;
break;
}
while (true);
}If you really want to specify player turn, you could adjust it like this:
std::string RPSPlayerChoice(int turn) {
cout > playerChoice;
do {
switch(playerChoice)
{
case 'r':
return "rock";
case 'p':
return "paper";
case 's':
return "scissors";
default:
cout << "Please make a valid choice" << std::endl;
break;
}
while (true);
}You could easily adjust this to be a string name, or any type you want that has an output operator.
One other thing I see is you always use
std::endl. std::endl flushes the output stream, which can make output unnecessarily expensive. It is better to force a new line with the '\n' new line character, and only flush once when you are done outputting:cout << "Player " << turn << ", please make your choice:\n";
cout << "Press (r) for rock, (p) for paper, or (s) for scissors" << std::endl;Code Snippets
cout << "Player 1, please make your choice:" << endl;
cout << "Press (r) for rock, (p) for paper, or (s) for scissors" << endl;
cin >> player1Choice;
switch(player1Choice)
{
case 'r':
p1->setChoice("rock");
break;
case 'p':
p1->setChoice("paper");
break;
case 's':
p1->setChoice("scissors");
break;
default:
cout << "Please make a valid choice" << endl;
break;
}std::string RPSPlayerChoice() {
cout << "Please make your choice:" << endl;
cout << "Press (r) for rock, (p) for paper, or (s) for scissors" << endl;
cin >> playerChoice;
do {
switch(playerChoice)
{
case 'r':
return "rock";
case 'p':
return "paper";
case 's':
return "scissors";
default:
cout << "Please make a valid choice" << endl;
break;
}
while (true);
}std::string RPSPlayerChoice(int turn) {
cout << "Player " << turn << ", please make your choice:" << std::endl;
cout << "Press (r) for rock, (p) for paper, or (s) for scissors" << std::endl;
cin >> playerChoice;
do {
switch(playerChoice)
{
case 'r':
return "rock";
case 'p':
return "paper";
case 's':
return "scissors";
default:
cout << "Please make a valid choice" << std::endl;
break;
}
while (true);
}cout << "Player " << turn << ", please make your choice:\n";
cout << "Press (r) for rock, (p) for paper, or (s) for scissors" << std::endl;Context
StackExchange Code Review Q#90746, answer score: 10
Revisions (0)
No revisions yet.