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

Simplification and efficiency suggestions for "Rock, Paper, Scissors" game

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

Problem

I wrote this program for an exercise in Bjarne Stroustrup's text Programming -- Principles and Practice Using C++. He recommended using a vector for pseudo randomness, however I opted for srand() and rand() from cstdlib.

Any recommendations regarding efficiency or simplification would be greatly appreciated. I apologize in advance for the nested switch statements. After much thought, I could not come up with a better solution. The exercise is as follows:


Write a program that plays the game "Rock, Paper, Scissors." If you are
not familiar with the game do some research (e.g., on the web using
Google). Research is a common task for programmers. Use a switch statement
to solve this exercise. Also, the machine should give random
answers (i.e., select the next rock, paper, or scissors randomly). Real randomness
is too hard to provide just now, so just build a vector with a sequence
of values to be used as "the next value." If you build the vector
into the program, it will always play the same game, so maybe you
should let the user enter some values. Try variations to make it less easy
for the user to guess which move the machine will make next.

#include 
#include 
#include 
#include 

void show_winner(char user_move, char comp_move)
{
    using namespace std;

    switch (user_move) {
        case 'r':
            switch (comp_move) {
                case 'r':
                    cout > move;
    cout > again;   
        if (again == "Y") {}
        else break;

        cout << endl;
    }           

    // Main Function Return
    return 0;       
}

Solution

Rather than have big switch statements. You use a matrix of valid results:

User Move ->       Rock      Paper      Scissor
   Comp Move  \/

            Rock      Tie       User-Win   Comp-Win
            Paper     Comp-Win  Tie        User-Win
            Scissor   User-Win  Comp-Win   Tie


This implies we should use enum

enum Weapon  {Rock, Paper, Scissor};
   enum Result  {Tie,  Comp_Win, User_Win};


To make printing it easier add the appropriate output operators:

std::ostream& operator<<(std::ostream& s, Weapon w)
   {
       return s << ((w == Rock) ? "Rock" : (w == Paper) ? "Paper" : "Scissor");
   }
   std::ostream& operator<<(std::ostream& s, Result r)
   {
       return s << ((r == Tie) ? "Tie" : (r == Comp_Win) ? "Comp_Win" : "User_Win");
   }


Now we can define the result matrix like this:

Result actionMatric[3][3] = [[Tie,      User_Win, Comp_Win],
                                [Comp_Win, Tie,      User_win],
                                [User_Win, Comp_Win, Tie]];

   void show_winner(Weapon user, Weapon comp)
   {
      // Notice don't use std::endl unless you want to flush the output.
      // If you use std::endl all the time you waste the buffer that is being used
      // to make the stream effecient.
      std::cout << "User:     " << user << "\n"
                << "Computer: " << comp << "\n"
                << std::endl;
       for (time_t t = time(0) + 1; time(0) < t;) {}   
       std::cout << actionMatric[user][comp] << std::endl;
   }


Don't use a busy wait() to sleep.

for (time_t t = time(0) + 1; time(0) < t;) {}

// Prefer:  one of these.
//          Usually for multi-platform code you macro out these
sleep(1);     // Unix
Sleep(1000);  // Win


Only seed the random number generator once in a game:

srand(time(NULL));


Easiest to right after main() is entered.

You don't need a switch to convert a random number to a value. You can calculate it (or with enum cast it).

Weapon compWeapon = static_cast(rand() % 3);


Don't do this:

using namespace std;

Code Snippets

User Move ->       Rock      Paper      Scissor
   Comp Move  \/

            Rock      Tie       User-Win   Comp-Win
            Paper     Comp-Win  Tie        User-Win
            Scissor   User-Win  Comp-Win   Tie
enum Weapon  {Rock, Paper, Scissor};
   enum Result  {Tie,  Comp_Win, User_Win};
std::ostream& operator<<(std::ostream& s, Weapon w)
   {
       return s << ((w == Rock) ? "Rock" : (w == Paper) ? "Paper" : "Scissor");
   }
   std::ostream& operator<<(std::ostream& s, Result r)
   {
       return s << ((r == Tie) ? "Tie" : (r == Comp_Win) ? "Comp_Win" : "User_Win");
   }
Result actionMatric[3][3] = [[Tie,      User_Win, Comp_Win],
                                [Comp_Win, Tie,      User_win],
                                [User_Win, Comp_Win, Tie]];

   void show_winner(Weapon user, Weapon comp)
   {
      // Notice don't use std::endl unless you want to flush the output.
      // If you use std::endl all the time you waste the buffer that is being used
      // to make the stream effecient.
      std::cout << "User:     " << user << "\n"
                << "Computer: " << comp << "\n"
                << std::endl;
       for (time_t t = time(0) + 1; time(0) < t;) {}   
       std::cout << actionMatric[user][comp] << std::endl;
   }
for (time_t t = time(0) + 1; time(0) < t;) {}

// Prefer:  one of these.
//          Usually for multi-platform code you macro out these
sleep(1);     // Unix
Sleep(1000);  // Win

Context

StackExchange Code Review Q#28212, answer score: 6

Revisions (0)

No revisions yet.