Object-oriented rock-paper-scissors game

Question

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 <iostream>
#include <string>
#include <ctime>
#include <cstdlib>
#include <vector>
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<Player> players(2);
void print_winner(int mode);
bool GAME_OVER();
void game_start(){
 std::cout << "-------------------------------------------\n";
 std::cout << "Select a mode: \n";
 std::cout << "1. human vs computer\n2. computer vs computer\n";
 std::cout << "-------------------------------------------\n";
 int mode;
 std::cin >> mode;
 if (mode == 1){
 std::cout << "Enter your name: ";
 std::cin >> 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 << "\nWhat will you chose? rock, paper, or scissors? ";
 std::cin >> players[0].hand;//player makes choice
 players[1].play();//computer makes its choice
 std::cout << std::endl;
 std::cout << "You chose: " << players[0].hand << " Computer chose: " << players[1].hand << std::endl; //display each player's choice for the round
 print_winner(1); //display the winner of the round
 while (players[0].hand == players[1].hand){ //check to see if the round was a draw
 std::cout << "\nWhat will you chose? rock, paper, or scissors? ";
 std::cin >> players[0].hand;
 players[1].play();
 std::cout << endl;
 std::cout << "You chose: " << players[0].hand << " Computer chose: " << players[1].hand << std::endl;
 print_winner(1);
 }
 }
 if (GAME_OVER()){ //display the winner of the game
 if (players[0].hands_won == 2){
 std::cout << "Congratulations, " << players[0].name << ". You win the game!\n";
 return 1;
 }else{
 std::cout << "Game over. You lose.\n";
 return 0;
 }
 }
 }
}
void print_winner(int mode){ //determine which player won the round
 if (mode == 1){
 if (players[0].hand == "rock" && players[1].hand == "scissors"){
 std::cout << " You win!";
 players[0].hands_won += 1;
 }else if (players[0].hand == "scissors" && players[1].hand == "paper"){
 std::cout << " You win!";
 players[0].hands_won += 1;
 }else if (players[0].hand == "paper" && players[1].hand == "rock"){
 std::cout << " You win!";
 players[0].hands_won += 1;
 }else if (players[0].hand == players[1].hand){
 std::cout << " It's a draw!";
 }else{
 std::cout << " Computer wins!";
 players[1].hands_won += 1;
 }
 }else{
 if (players[0].hand == "rock" && players[1].hand == "scissors"){
 std::cout << " Computer 1 wins!";
 players[0].hands_won += 1;
 }else if (players[0].hand == "scissors" && players[1].hand == "paper"){
 std::cout << " Computer 1 wins!";
 players[0].hands_won += 1;
 }else if (players[0].hand == "paper" && players[1].hand == "rock"){
 std::cout << " Computer 1 wins!";
 players[0].hands_won += 1;
 }else if (players[0].hand == players[1].hand){
 std::cout << " It's a draw!";
 }else{
 std::cout << " Computer 2 wins!";
 players[1].hands_won += 1;
 }
 }
}
int main()
{ 
 std::srand(time(NULL));
 std::cout << "---------------------------------------------------\n";
 std::cout << "Welcome to Rock, Paper, Scissors.\nTo play, type '1'. \nTo exit, type '2'.\n";
 std::cout << "---------------------------------------------------\n";
 int choice;
 std::cin >> choice;
 switch (choice){
 case 1:
 game_start();
 break;
 case 2:
 return 0;
 default:
 std::cout << "You made an invalid choice. Exiting game...";
 return 0;
 }
 if (GAME_OVER()){ //ask the user if they would like to play again
 std::cout << "Would you like to play again? <y/n> ";
 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;
}

• David St Pierre: I had actually answered a post about how to perform the y or n to exit program. I suggest you take a look at it if you would like to utilize 'y' or 'n' instead of 'y' or anything else.
• Fugs: Thanks for pointing that out! I actually just forgot to finish it.

Answer 1

Things that definitely need fixing

1. Line 52 has:

    std::cout << endl;
   

   That is compiler error. It needs to be:

    std::cout << std::endl;
   

2. 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 <iostream>
#include <string>
#include <ctime>
#include <cstdlib>
#include <limits>
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 << "\nWhat will you chose? rock, paper, or scissors? ";
 std::cin >> hand;//player makes choice
 }
 virtual void printWinningMessage()
 {
 std::cout << "You win!" << std::endl;
 }
};
class ComputerPlayer : public Player
{
 public:
 ComputerPlayer(std::string const& name) : Player(name) {}
 void play()
 {
 std::string choices[3]{"rock", "paper", "scissors"};
 hand = choices[std::rand()%3]; //select rock, paper, or scissors at random
 }
 virtual void printWinningMessage()
 {
 std::cout << name << " wins!" << std::endl;
 }
};
enum TopLevelChoice { PLAY_GAME, EXIT_GAME };
enum PlayMode { HUMAN_VS_COMPUTER, COMPUTER_VS_COMPUTER };
TopLevelChoice get_toplevel_choice();
PlayMode get_play_mode();
void play_game();
void play_game(Player& player1, Player& player2);
void play_human_vs_computer();
void play_computer_vs_computer();
void pick_round_winner(Player& player1, Player& player2);
void print_winner(Player& player1, Player& player2);
bool is_game_over(Player& player1, Player& player2);
int main()
{ 
 std::srand(time(NULL));
 while ( get_toplevel_choice() != EXIT_GAME )
 {
 play_game();
 }
 return 0;
}
TopLevelChoice get_toplevel_choice()
{
 std::cout << "---------------------------------------------------\n";
 std::cout << "Welcome to Rock, Paper, Scissors.\nTo play, type '1'. \nTo exit, type '2'.\n";
 std::cout << "---------------------------------------------------\n";
 int choice;
 std::cin >> choice;
 if ( std::cin ) // Reading was successful.
 {
 switch (choice)
 {
 case 1:
 return PLAY_GAME;
 case 2:
 return EXIT_GAME;
 default:
 std::cout << "You made an invalid choice. Try again...\n";
 }
 return get_toplevel_choice();
 }
 else
 {
 std::cout << "You made an invalid choice. Try again...\n";
 // Clear the stream
 std::cin.clear();
 // Skip bad input
 std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
 // Try again.
 return get_toplevel_choice();
 }
}
PlayMode get_play_mode()
{
 std::cout << "-------------------------------------------\n";
 std::cout << "Select a mode: \n";
 std::cout << "1. human vs computer\n2. computer vs computer\n";
 std::cout << "-------------------------------------------\n";
 int mode;
 std::cin >> mode;
 if ( std::cin ) // Reading was successful.
 {
 switch (mode)
 {
 case 1:
 return HUMAN_VS_COMPUTER;
 case 2:
 return COMPUTER_VS_COMPUTER;
 default:
 std::cout << "You made an invalid choice. Try again...\n";
 }
 return get_play_mode();
 }
 else
 {
 std::cout << "You made an invalid choice. Try again...\n";
 // Clear the stream
 std::cin.clear();
 // Skip bad input
 std::cin.ignore(std::numeric_limits<std::streamsize>::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 << std::endl;
 std::cout << player1.name << " chose : " << player1.hand << std::endl;
 std::cout << player2.name << " chose : " << player2.hand << std::endl;
 if (player1.hand == player2.hand) 
 {
 // The round was a draw 
 // Play next round
 std::cout << " It's a draw!";
 continue;
 }
 // Pick the winner of this round
 pick_round_winner(player1, player2);
 } while ( !is_game_over(player1, player2));
 print_winner(player1, player2);
}
void play_human_vs_computer()
{
 std::string name;
 std::cout << "Enter your name: ";
 std::cin >> 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);
}

• Miklas: Three more tips on your refactoring (and of course on @Fugs original code). First, try to be const correct. Functions that do not modify the object (is_game_over, print_winner ...) can use const arguments. Secondly, string comparisons are more expensive than enum comparisons. Why not use an enum for Rock/Paper/Scissor instead of a string? Thirdly, try to use a mathematical solution instead of brute force. Pick_round_winner has a few if's, but what if it becomes a rock-paper-scissors-lizard-Spock edition?

Answer 2

Can't say much, it is easily readable. My suggestion is, instead of std::cout << "You made an invalid choice. Exiting game..."; return 0; , loop it back to ask again. Perhaps a user mistakenly entered the wrong directive, it would be irritating to reopen the application.

Other than that, good job OP :)

• Fugs: Thats a good idea. I thought about doing that at first, but then the laziness kicked in. I will definitely go back and add that!

Answer 3

Two things I see, first personally I like to add a constructor for all classes, if for no other reason then insure all member variables are in a consistent, reasonable state. Secondly, I would make all of the member variables private. This will enforce data-encapsulation, although you will need to write a few other functions.

While neither of these suggestions are critical for a small program, as you move to larger and larger programs, doing both of these will be come more important.

 class Player{
 public:
 Player() : name(""), hand(""), hands_won(0)
 { }
 // getter functions
 std::string getName() {return name;}
 std::string getHand() {return hand;}
 int handsWon() {return hands_won;}
 // setter functions
 void setName(std::string n) {name = n;}
 void setHand(std::string h) {hand = h;}
 void incHandsWon() {++hands_won;}
 void play(); //for the computer players only
 private:
 std::string name;
 std::string hand;
 int hands_won;
 };

Other than these, the code looks pretty good.