# Single player Rock Paper Scissors game

Here is the RPS game I made. I watched a 4 hour tutorial about C++ from freecodeacademy and produced this game right after it. If you have any feedback or suggestions please let me know. Or if there's anything I should be aware of or shouldn't be doing that would be great.

#include <iostream>
#include <conio.h>
#include <time.h>
#include <stdlib.h>

using namespace std;

int rng() {
srand(time(NULL));
return rand() % 3 + 1;
}
int main() {

cout << userName << " how many rounds of Rock Paper Scissors would you like to play?" << endl;
int rounds;
cin >> rounds;
int preRound = 0;

int userScore = 0;
int computerScore = 0;
int tie = 0;

while (preRound != rounds) {

cout << "Please make a selection!" << endl;
cout << "1.) Rock " << endl;
cout << "2.) Paper " << endl;
cout << "3.) Scissors " << endl;
int userChoice;
cin >> userChoice;
int computerChoice = rng();
switch (userChoice) {
case 1:
cout << "*You chose Rock*" << endl;
cout << endl;
break;
case 2:
cout << "*You chose Paper*" << endl;
cout << endl;
break;
case 3:
cout << "*You chose Scissors*" << endl;
cout << endl;
break;
default:
cout << "**Invalid choice, please try again!**" << endl;
cout << endl;
main();
break;
}
switch (computerChoice) {
case 1:
cout << "|The computer chose Rock|" << endl;
cout << endl;
break;
case 2:
cout << "|The computer chose Paper|" << endl;
cout << endl;
break;
case 3:
cout << "|The computer chose Scissors|" << endl;
cout << endl;
break;
}
if (computerChoice == userChoice) {
cout << "The game results in a tie!" << endl;
tie++;
cout << "Your Score = " << userScore << endl;
cout << "Computer Score = " << computerScore << endl;
cout << "ties = " << tie << endl;
cout << "\n";
preRound++;
}
else if (computerChoice == 1 && userChoice == 2) { // 1 = Rock, 2 = Paper, 3 = Scissors
cout << userName << " wins the game!" << endl;
userScore++;
cout << "Your Score = " << userScore << endl;
cout << "Computer Score = " << computerScore << endl;
cout << "ties = " << tie << endl;
cout << "\n";
preRound++;
}
else if (computerChoice == 3 && userChoice == 1) {
cout << userName << " wins the game!" << endl;
userScore++;
cout << "Your Score = " << userScore << endl;
cout << "Computer Score = " << computerScore << endl;
cout << "ties = " << tie << endl;
cout << "\n";
preRound++;
}
else if (computerChoice == 2 && userChoice == 3) {
cout << userName << " wins the game!" << endl;
userScore++;
cout << "Your Score = " << userScore << endl;
cout << "Computer Score = " << computerScore << endl;
cout << "ties = " << tie << endl;
cout << "\n";
preRound++;
}////////////////////////////////////////////////
else if (computerChoice == 2 && userChoice == 1) {
cout << "The computer wins!" << endl;
computerScore++;
cout << "Your Score = " << userScore << endl;
cout << "Computer Score = " << computerScore << endl;
cout << "ties = " << tie << endl;
cout << "\n";
preRound++;
}
else if (computerChoice == 1 && userChoice == 3) {
cout << "The computer wins!" << endl;
computerScore++;
cout << "Your Score = " << userScore << endl;
cout << "Computer Score = " << computerScore << endl;
cout << "ties = " << tie << endl;
cout << "\n";
preRound++;
}
else if (computerChoice == 3 && userChoice == 2) {
cout << "The computer wins!" << endl;
computerScore++;
cout << "Your Score = " << userScore << endl;
cout << "Computer Score = " << computerScore << endl;
cout << "ties = " << tie << endl;
cout << "\n";
preRound++;
}
}
return 0;
}


### Use C++ standard library headers

Don't use C headers such as <stdlib.h>, use the equivalent C++ headers like <cstdlib>. You should be using the C++ library functions anyway.

### Don't rely on transitive includes

It happens that <iostream> will include <string> itself in most implementations, which is why you can use string without including <string>. However, you shouldn't rely on this as it's not guaranteed to always work. Include all the headers needed for the components you're using.

### Never use using namespace std;

This is a bad idea for a number of reasons. See this post for an in-depth discussion of why this is the case. Instead, simply prefix all the names that you're using from the std namespace with std::, so write std::string instead of string, etc.

### Use modern random number generation facilities

rand is not particularly random. While it may suffice for a simple toy game, you should get out of the habit of using it. You're also doing it a bit incorrectly; see this post for what you need to know if you're calling srand and rand one after the other multiple times.

Instead of using rand, you should use the tools in the <random>header. The separation into a generator, and a distribution might seem like more work, but is absolutely crucial in order to implement randomness well.

### Don't use cin >> userName; to read in names

std::cin will only read in strings up to the first whitespace. This means the program won't work if someone puts in their full name. Instead, use std::getline which by default will read in all the characters until the user presses enter.

std::getline(std::cin, userName);


### Refactor your code into functions

The goal should be to have every function do exactly one thing, and it shouldn't be too long. There's no exact size of function that you need to go for, you'll get a sense of what is a reasonable level of complexity for a function with practice. At the moment, apart from the rng function, everything is in the main function, which is too much complexity for a single function.

I'd suggest adding at least 2 more functions. One could be called something like makeChoice which basically contains the code asking the user to make the choice, along with the two switch statements. The second function could be decideWinner which contains the nested if-else statements.

### Don't repeat code

Your nested if-else statements are repeating a lot of code. The clearest example is preRound++; which is executed in every branch. As soon as you see that, you should take that statement out of the if-else blocks and write it only once.

There are basically 3 options for who wins a particular round, not 7 as you've written. This options are 1. Tie, 2. User wins, 3. Computer wins. Note that this is why the code in branches 2,3, and 4 are the same as each other, as well as the code in branches 5,6, and 7. If you group those checks together, the code would look something like:

if (computerChoice == userChoice) {
// ... Tie
}
else if ((computerChoice == 1 && userChoice == 2)
or (computerChoice == 3 && userChoice == 1)
or (computerChoice == 2 && userChoice == 3)) {
// ... User wins
}
else {  // nothing to check if code reaches here since computer *must* win :)
// ... Computer wins
}

• It will be much better if you also show your complete code substituting for the OP's. – Kim Jong Un Mar 26 at 6:21