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

Object-oriented rock-paper-scissors game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
paperscissorsgamerockorientedobject

Problem

This is my code for a simple rock-paper-scissors game. Users have the choice between playing against a computer or watching a "computer player" play another "computer player".

The code for the computer vs computer mode isn't finished yet, so you must chose human vs computer mode for the program to work as expected.

If you guys have any suggestions, corrections, recommendations, etc., please share them.

#include 
#include 
#include 
#include 
#include 

class Player{
        public:
        void play(); //for the computer players only
        std::string hand;
        int hands_won;
        std::string name;
};
void Player::play(){
    std::string choices[3]{"rock", "paper", "scissors"};
    hand = choices[std::rand()%3]; //select rock, paper, or scissors at random
}
std::vector players(2);
void print_winner(int mode);
bool GAME_OVER();
void game_start(){
    std::cout > mode;
    if (mode == 1){
        std::cout > players[0].name;
        players[1].name = "Computer";
    }
    if (mode == 3){
        players[0].name = "Computer 1";
        players[1].name = "Computer 2";
    }
    while (mode == 1){ //game loop for mode 1
        while (!GAME_OVER()){
            std::cout > players[0].hand;//player makes choice
            players[1].play();//computer makes its choice
            std::cout > players[0].hand;
                players[1].play();
                std::cout > choice;
    switch (choice){
        case 1:
            game_start();
            break;
        case 2:
            return 0;
        default:
            std::cout    ";
        char input;
        std::cin >> input;
        if (input == 'y'){
            game_start();
        }
        return 0;
    }
}
bool GAME_OVER(){
    if(players[0].hands_won == 2 || players[1].hands_won == 2){ //best 2/3
        return true;
    }
    return false;
}

Solution

Things that definitely need fixing

-
Line 52 has:

std::cout << endl;


That is compiler error. It needs to be:

std::cout << std::endl;


-
You have lines 60 and 63:

return 1;
 return 0;


These lines are in a function that has void as return type. These also produce compile errors. They need to be changed to:

return;
 return;


Inconsistent naming convention

All of your functions are consistently named except GAME_OVER. That should be changed to game_over to be consistent.

Inconsistent physical layout

All of your functions are declared and defined before main except GAME_OVER which is defined after main. I would recommend putting the definitions of all the helper function before main or all of them after main. Having some of them before main and some of them after main is inconsistent.

Personally, I prefer the declarations of all the functions to be at the top of the file irrespective of whether they are defined before main or after main.

Names of functions

This maybe a personal preference, but I think function names that start with a verb seem better than those that start with a noun.

Use start_game instead of game_start

Use is_game_over instead of game_over

print_winner already starts with a verb.

Using enums for user choices

Instead of using magic numbers like 1 and 2, I would recommend using enums. That will make the code more readable.

enum TopLevelChoice { PLAY_GAME, EXIT_GAME };
enum PlayMode { HUMAN_VS_COMPUTER, COMPUTER_VS_COMPUTER };


Make functions still smaller

You have functions that accept input, process the input, and display the result of processing the input. You can separate the first part the last part into separate functions. That will make your code easier to follow and maintain. For example, main can be:

int main()
{   
   std::srand(time(NULL));
   while ( get_toplevel_choice() != EXIT_GAME )
   {
      play_game();
   }
   return 0;
}


Make Player a base class

You can simplify the code in a few places by making Player a base class and creating two sub-classes: HumanPlayer and ComputerPlayer.

Refactored Program

#include 
#include 
#include 
#include 
#include 

class Player
{
   public:

      Player(std::string const& name) : name(name), hands_won(0) {}

      virtual ~Player() {}

      virtual void play() = 0;

      virtual void printWinningMessage() = 0;

      std::string hand;
      std::string name;
      int hands_won;
};

class HumanPlayer : public Player
{
   public:

      HumanPlayer(std::string const& name) : Player(name) {}

      virtual void play()
      {
         std::cout > hand;//player makes choice
      }

      virtual void printWinningMessage()
      {
         std::cout > choice;
   if ( std::cin ) // Reading was successful.
   {
      switch (choice)
      {
         case 1:
            return PLAY_GAME;
         case 2:
            return EXIT_GAME;
         default:
            std::cout ::max(), '\n');

      // Try again.
      return get_toplevel_choice();
   }
}

PlayMode get_play_mode()
{
   std::cout > mode;
   if ( std::cin ) // Reading was successful.
   {
      switch (mode)
      {
         case 1:
            return HUMAN_VS_COMPUTER;
         case 2:
            return COMPUTER_VS_COMPUTER;
         default:
            std::cout ::max(), '\n');

      // Try again.
      return get_play_mode();
   }
}

void play_game()
{
   PlayMode choice = get_play_mode();
   if ( choice == HUMAN_VS_COMPUTER )
   {
      play_human_vs_computer();
   }
   else
   {
      play_computer_vs_computer();
   }
}

void play_game(Player& player1, Player& player2)
{
   do
   {
      player1.play();
      player2.play();

      //display each player's choice for the round
      std::cout > name;

   HumanPlayer humanPlayer(name);
   ComputerPlayer computerPlayer("Computer");
   play_game(humanPlayer, computerPlayer);
}

void play_computer_vs_computer()
{
   ComputerPlayer computerPlayer1("Computer 1");
   ComputerPlayer computerPlayer2("Computer 2");
   play_game(computerPlayer1, computerPlayer2);
}

void pick_round_winner(Player& player1, Player& player2)
{
   // We have already taken care of the draw.
   if  ( (player1.hand == "rock" && player2.hand == "scissors") ||
         (player1.hand == "scissors" && player2.hand == "paper") ||
         (player1.hand == "paper" && player2.hand == "rock") )
   {
      player1.printWinningMessage();
      player1.hands_won += 1;
   }
   else
   {
      player2.printWinningMessage();
      player2.hands_won += 1;
   }
}

void print_winner(Player& player1, Player& player2)
{
   Player& winner = (player1.hands_won == 2) ? player1 : player2;
   std::cout << "Congratulations, " << winner.name << ". You win the game!\n";
}

bool is_game_over(Player& player1, Player& player2)
{
   return (player1.hands_won == 2 || player2.hands_won == 2);
}

Code Snippets

std::cout << endl;
std::cout << std::endl;
return 1;
 return 0;
return;
 return;
enum TopLevelChoice { PLAY_GAME, EXIT_GAME };
enum PlayMode { HUMAN_VS_COMPUTER, COMPUTER_VS_COMPUTER };

Context

StackExchange Code Review Q#90588, answer score: 5

Revisions (0)

No revisions yet.