HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Rock-paper-scissors Game in C++

Submitted by: @import:stackexchange-codereview··
0
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

#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;
};
#endif


Player.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 - 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.