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

Rock Paper Scissors C++ Game

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

Problem

I just started to learn C++ and I decided to make a Rock Paper Scissors game. It works fine but I would like to know how can I improve it further.
In particular I couldn't find an efficient way to implement a play again feature. Thanks in advance.

```
#include
#include
#include
#include

using namespace std;

int main() {

srand(static_cast(time(NULL)));

string userChoice;
int computerNumber = rand() % 3 + 1;
string computerChoice;
string userName;

if (computerNumber == 1) {
computerChoice = "rock";
} else if (computerNumber == 2) {
computerChoice = "paper";
} else {
computerChoice = "scissors";
}

std::cout << "Welcome to rock, paper, scissors! What is your name?" << endl;
getline(cin, userName);
std::cout << "Hello " << userName << "! What do you choose?" << endl;
getline (cin,userChoice);

for (int i=0;i<userChoice.length();i++){
userChoice[i]=tolower(userChoice[i]);
}

while (userChoice != "rock" && userChoice != "paper" && userChoice != "scissors") {
std::cout << "Sorry! Didn't get that. Please enter again." << endl;
getline(cin, userChoice);
for (int i=0;i<userChoice.length();i++){
userChoice[i]=tolower(userChoice[i]);
}
}

std::cout << "Press enter to continue..." << endl;
std::cin.ignore();

if(computerChoice == "rock" && userChoice == "rock") {
std::cout << "It was a tie!" << endl;
} else if (computerChoice == "rock" && userChoice == "scissors") {
std::cout << "The computer won! Better luck next time!" << endl;
} else if (computerChoice == "paper" && userChoice == "paper") {
std::cout << "It was a tie!" << endl;
} else if (computerChoice == "paper" && userChoice == "rock") {
std::cout << "The computer won! Better luck next time!" << endl;
} else if (computerChoice == "scissors" && userChoice == "scissors") {
std::cout << "It was a tie!" << endl;
} else if (computerChoice == "scissors" && userChoice == "paper") {
std::cout << "The computer won! Better luck next time!" << endl;
} else {

Solution

Indententation

Normally code is indented between { and }

int main()
{
    // Start here
}


Namespace

Never Do this.

using namespace std;


If you had read any other C++ review on this board it would have told you not to do this. See: Why is “using namespace std” in C++ considered bad practice?.

There is a reason that the standard libraries are called std and not standard. Its to make prefixing them easy.

Rand

Though your use of srand() and rand() is perfectly valid. This version of the random number generated is now obsolete. You should look up how to use the new random number functionality provided by C++11.

https://stackoverflow.com/a/14009667/14065

https://stackoverflow.com/a/19666713/14065

Note:

int computerNumber = rand() % 3 + 1;


This is slightly biased against 3. Assume RAND_MAX is 32768.

If you rolled 32768 times. Then your distribution would be.

1 10923/32768   
2 10923/32768
3 10922/32768  <- Notice this is one smaller.


Invalid:

} else {
    std::cout << "Congrats! You won!" << endl;
}


Do you really want to have an invalid choice. You have already enumerated all the valid choices. If you get to this point there is a serious error in your code. So you need to indicate this (so you can catch it in testing).

else
{
    throw std::runtime_error("Error");
}


Data Driven Programming

Rather than write code to decide the winner:

if(computerChoice == "rock" && userChoice == "rock") {
    std::cout << "It was a tie!" << endl;
} else if (computerChoice == "rock" && userChoice == "scissors") {
    std::cout << "The computer won! Better luck next time!" << endl;
} else if (computerChoice == "paper" && userChoice == "paper") {
    std::cout << "It was a tie!" << endl;
} else if (computerChoice == "paper" && userChoice == "rock") {
    std::cout << "The computer won! Better luck next time!" << endl;
} else if (computerChoice == "scissors" && userChoice == "scissors") {
    std::cout << "It was a tie!" << endl;
} else if (computerChoice == "scissors" && userChoice == "paper") {
    std::cout << "The computer won! Better luck next time!" << endl;
} else {
    std::cout << "Congrats! You won!" << endl;
}


Use a data structure. Assume you translate the choices into a number for each user.

0 -> rock
1 -> paper
2 -> scissor

// This array decides if who is the winner.
// 0 => Draw.
// 1 => Player 1
// 2 => Player 2
int winner[3][3] = {{0, 2, 1},
                    {1, 0, 2},
                    {2, 1, 0}};

// Then to decide the winner.
int theWinner = winner[player1Choice][player2Choice];


Advice


In particular I couldn't find an efficient way to implement a play again feature.

To make things like retrying the code you need to start using functions so that you can logically compartmentalize your code.

int main()
{
    std::string name = getPlayerName();
    do
    {
        int player1 = getComputerMove();
        int player2 = getPlayerMove();
        displayWinner(player1, player2);
    }
    while(doesPlayerWantToContinue());
}

Code Snippets

int main()
{
    // Start here
}
using namespace std;
int computerNumber = rand() % 3 + 1;
If you rolled 32768 times. Then your distribution would be.

1 10923/32768   
2 10923/32768
3 10922/32768  <- Notice this is one smaller.
} else {
    std::cout << "Congrats! You won!" << endl;
}

Context

StackExchange Code Review Q#144013, answer score: 7

Revisions (0)

No revisions yet.