Need assistance with simplifying rock paper scissors game.

[Nickb70]

I am trying to make a program for rock paper scissors, and this is what I have come up with. It is complete and works, but seems far too inefficient. How can I make this code more simple and efficient? I feel like this is way too much code for a simple game, but I'm not sure what to change. Any advice and tips would be greatly appreciated.


Here's the code:

 #include <iostream>

using namespace std;

class RockPaperScissors{
public:
	void getCPUChoice();
	void gameOutcome();
	void getChoice();
private:
	int cpuChoice;
	char rps, cpuRps;
};
void RockPaperScissors::gameOutcome(){
	if (rps == 'R') cout << "You chose rock" << endl;
	else if (rps == 'P') cout << "You chose paper" << endl;
	else if (rps == 'S') cout << "You chose scissors" << endl;

	if (cpuRps == 'R') cout << "CPU chose rock" << endl;
	else if (cpuRps == 'P') cout << "CPU chose paper" << endl;
	else if (cpuRps == 'S') cout << "CPU chose scissors" << endl;

	if (rps == 'R' && cpuRps == 'R' || rps == 'P' && cpuRps == 'P' || rps == 'S' && cpuRps == 'S') cout << "\nIt's a draw\n";
	else if (rps == 'R' && cpuRps == 'S' || rps == 'S' && cpuRps == 'P' || rps == 'P' && cpuRps == 'R') cout << "\nYou won!\n";
	else if (rps == 'S' && cpuRps == 'R' || rps == 'P' && cpuRps == 'S' || rps == 'R' && cpuRps == 'P') cout << "/nCPU won!\n";
	system("pause");
}
void RockPaperScissors::getCPUChoice(){
	srand((unsigned)time(0));
	cpuChoice = (rand() % 3) + 1;
	if (cpuChoice == 1) cpuRps = 'R';
	else if (cpuChoice == 2) cpuRps = 'P';
	else if (cpuChoice == 3) cpuRps = 'S';
}

void RockPaperScissors::getChoice(){
	system("CLS");
	cout << "Rock, Paper, or Scissors(R,P,S)? ";
	cin >> rps;
	cout << endl;
}

[Thomas1965]

I wrote games like this in C and also in C++ - though with a different approach.
If you want to have a look, maybe you find sth. useful in them.
https://www.planet-source-code.com/vb/scripts/BrowseCategoryOrSearchResults.asp?lngWId=3&blnAuthorSearch=TRUE&lngAuthorId=442548893&strAuthorName=Thomas1965&txtMaxNumberOfEntriesPerPage=25

[SamuelAdams]

Is there anything you don't need ?
and it works ? didn't for me..

There are a dozen ways to write this. You did it in 40 lines.
What would make you happy ? 35 lines ?

[icy1]

here, have a hacky solution

#include <iostream>
#include <limits>
#include <string>
using namespace std;

int main() 
{
    enum Symbol { ROCK=1, PAPER=2, SCISSORS=4 };
    const string STR[] {"","Rock","Paper","","Scissors"};
    srand(time(0));
    
    cout << "[r] " << STR[ROCK] << endl << 
            "[p] " << STR[PAPER] << endl <<
            "[s] " << STR[SCISSORS] << endl << endl <<
            "[q] Quit" << endl;
    char c;
    Symbol s;
    int cr, result;    
    while (true)
    {        
        cout << "? ";
        cin >> c;        
        cin.ignore(numeric_limits<streamsize>::max(), '\n');
        if (c=='r')      s = ROCK;
        else if (c=='p') s = PAPER;
        else if (c=='s') s = SCISSORS;
        else if (c=='q') break;
        else             continue;
        
        cr = rand() % 3 + 1;
        if (cr==3) 
            cr+=1;
        cout << "Your " << STR[s];
        result = s|cr;
        if (result == s)  
            cout << " matches ";
        else if ( (result == 3 && s<cr) ||
                  (result == 5 && s>cr) ||
                  (result == 6 && s<cr) ) 
            cout << " loses to ";
        else 
            cout << " defeats ";
        cout << "Computer's " << STR[cr] << endl;
    }
    cout << endl << "See ya!" << endl;

    return 0;
}

https://repl.it/@icy_1/DeadEnragedBusiness

[tpb]

The biggest improvement you could make is to get rid of the totally pointless class. It's just a way of using global variables while pretending not to. None of your functions take any parameters or return anything. It's just silly.

srand should only be called once in the program.
You should include all headers that you require (e.g., <cstdlib> for system, rand, srand).

#include <iostream>
#include <cstdlib>
#include <ctime>
#include <cctype>
using namespace std;

const char *object(char rps) {
    switch (rps) {
    case 'R': return "rock";
    case 'P': return "paper";
    case 'S': return "scissors";
    }
    return "<unknown>";
}

void gameOutcome(char rps, char cpuRps){
    cout << "\nYou chose " << object(rps) << '\n';
    cout << "CPU chose " << object(cpuRps) << '\n';
    if (rps == cpuRps)
        cout << "\nIt's a draw\n";
    else if ((rps == 'R' && cpuRps == 'S') ||
             (rps == 'S' && cpuRps == 'P') ||
             (rps == 'P' && cpuRps == 'R'))
        cout << "\nYou won!\n";
    else
        cout << "\nCPU won!\n";
}

char getCPUChoice(){
    switch (rand() % 3) {
    case 0: return 'R';
    case 1: return 'P';
    case 2: return 'S';
    }
    return 0; // not reached
}

char getChoice(){
    char rps = 0;
    do {
        cout << "\nRock, Paper, Scissors or Quit (R,P,S,Q)? ";
        cin >> rps;
        rps = toupper(rps);
    } while (rps != 'R' && rps != 'P' && rps != 'S' && rps != 'Q');
    return rps;
}

int main() {
    srand(unsigned(time(0)));

    while (true) {
        char rps = getChoice();
        if (rps == 'Q')
            break;
        char cpuRps = getCPUChoice();
        gameOutcome(rps, cpuRps);
    }
}

[jonnin]

What if you had a table of human vs ai?  Let the human be rows... and Win/Lose/Draw (these could be strings, and probably should be)

   RPS
______
R|DLW
P|WDL
S|LWD

the its simply 
cin human
roll AI
cout << table[humanentry][aichoice];

with a few statements to do some additional menu/output/etc stuff.