I am getting all of my outputs instead of just one

Question

Write a program to score the paper-rock-scissor game. Each of two users types in either P, R, or S. The program then announces the winner as well as the basis for determining the winner: Paper covers rock, Rock breaks scissors, Scissors cut paper, or Nobody wins. Be sure to allow the users to use lowercase as well as uppercase letters.

//System Libraries
#include <iostream> //Input/Output Library
#include <iomanip>
#include <string>
using namespace std;

//User Libraries

//Global Constants, no Global Variables are allowed
//Math/Physics/Conversions/Higher Dimensions - i.e. PI, e, etc...

//Function Prototypes

//Execution Begins Here!
int main(int argc, char** argv) {
    //Set the random number seed

    //Declare Variables
    float 
           P,p,
           R,r,
           S,s,
           p1,p2;

    //Initialize or input i.e. set variable values
    cout<<"Rock Paper Scissors Game\n";
    cout<<"Input Player 1 and Player 2 Choices\n";
    cin>>p1;
    cin>>p2;
    //Map inputs -> outputs
    if (p1 == p2)
    cout<<"tie";

    if ((p1 == P) && (p2 == R))
     cout<<"Paper covers rock.";
    if ((p1 == p) && (p2 == r))
     cout<<"Paper covers rock.";
    if ((p1 == P) && (p2 == r))
     cout<<"Paper covers rock.";
    if ((p1 == p) && (p2 == R))
     cout<<"Paper covers rock.";

    if ((p1 == S) && (p2 == R))
        cout<<"Rock breaks scissors.";
    if ((p1 == s) && (p2 == r))
        cout<<"Rock breaks scissors.";
    if ((p1 == S) && (p2 == r))
        cout<<"Rock breaks scissors.";
    if ((p1 == s) && (p2 == R)) 
        cout<<"Rock breaks scissors.";

    if ((p1 == S) && (p2 == P)) 
        cout<<"Scissors cut paper."; 
    if ((p1 == s) && (p2 == p)) 
        cout<<"Scissors cut paper."; 
    if ((p1 == S) && (p2 == p)) 
        cout<<"Scissors cut paper."; 
    if ((p1 == s) && (p2 == P)) 
        cout<<"Scissors cut paper."; 

    //Display the outputs

    //Exit stage right or left!
    return 0;
}

Expected:

Rock Paper Scissors Game
Input Player 1 and Player 2 Choices
Paper covers rock

MY RESULTS:

Rock Paper Scissors Game
Input Player 1 and Player 2 Choices
rs
tiePaper covers rock.Paper covers rock.Paper covers rock.Paper covers rock.Rock breaks scissors.Rock breaks scissors.Rock breaks scissors.Rock breaks scissors.Scissors cut paper.Scissors cut paper.Scissors cut paper.Scissors cut paper.

I'd recommend you start with some simplifications. You created a lot of work for yourself by testing each letter for both lowercase and uppercase variants. Simply convert the input the user gives to uppercase so you don't have to test for lowercase variants. `toupper` will do this you must include `cctype`. Make sure you read the documentation (cannot stress enough). You'll want to change those `float`s to be `unsigned int`s instead. — Dustin Nieffenegger, Jan 16 '19 at 05:48
Obviously p1 == p2 == P == p == R == r == S == s, please learn [how to debug c++ program with break point](https://learn.microsoft.com/en-us/visualstudio/debugger/using-breakpoints?view=vs-2017) — shingo, Jan 16 '19 at 05:54
About [`using namespace::std`](https://stackoverflow.com/q/1452721/1312382)... — Aconcagua, Jan 16 '19 at 06:28

gameon67 · Accepted Answer · 2019-01-16T07:10:24.047

1

You can use tolower instead comparing letter case
And also use char instead of float
You only have to declare user input variable
Use else if

Code :

#include <iostream> //Input/Output Library
#include <iomanip>
#include <string>
using namespace std;

//User Libraries

//Global Constants, no Global Variables are allowed
//Math/Physics/Conversions/Higher Dimensions - i.e. PI, e, etc...

//Function Prototypes

//Execution Begins Here!
int main(int argc, char** argv) {
    //Set the random number seed

    //Declare Variables
    char p1,p2;

    //Initialize or input i.e. set variable values
    cout<<"Rock Paper Scissors Game\n";
    cout<<"Input Player 1 Choice\n";
    cin>>p1;
    cout<<"Input Player 2 Choice\n";
    cin>>p2;
    //Map inputs -> outputs
    if (p1 == p2)
        cout<<"tie";
    else if ((tolower(p1) == 'p') && (tolower(p2) == 'r'))
        cout<<"Paper covers rock.";
    else if ((tolower(p2) == 'p') && (tolower(p1) == 'r'))
        cout<<"Paper covers rock.";

    //or combine above 2 statements to 
    //else if (((tolower(p1) == 'p') && (tolower(p2) == 'r')) || ((tolower(p2) == 'p') && (tolower(p1) == 'r')))
        //cout<<"Paper covers rock.";

    else if ((tolower(p1) == 's') && (tolower(p2) == 'r'))
        cout<<"Rock breaks scissors.";
    else if ((tolower(p2) == 's') && (tolower(p1) == 'r'))
        cout<<"Rock breaks scissors.";
    else if ((tolower(p1) == 's') && (tolower(p2) == 'p'))
        cout<<"Scissors cut paper.";
    else if ((tolower(p2) == 's') && (tolower(p1) == 'p'))
        cout<<"Scissors cut paper.";

    //Display the outputs

    //Exit stage right or left!
    return 0;
}

edited Jan 16 '19 at 07:10

answered Jan 16 '19 at 05:55

gameon67

3,981
5
35
61

okay I copied this into netbeans 10 and I am now getting no output – Dylan Yazdani Jan 16 '19 at 06:01
I just changed the input method, so first you input the first player's choice, hit enter then you input the second player's choice. I just prefer that way – gameon67 Jan 16 '19 at 06:03
I am still getting a blank output now. It seems that I keep getting blank outputs when using char – Dylan Yazdani Jan 16 '19 at 06:06
can u post your input? – gameon67 Jan 16 '19 at 06:08
posted it below – Dylan Yazdani Jan 16 '19 at 06:10
@DylanYazdani try it again, I have updated my answer, I added more if statement – gameon67 Jan 16 '19 at 06:12
yes thank you so much! I got confused and made it way too complex. – Dylan Yazdani Jan 16 '19 at 06:20
literally, my first time using this website so I have no idea how to do that. If you tell me I can do it – Dylan Yazdani Jan 16 '19 at 06:23
1

Several pairs of different if-conditions result in the same body - you *could* combine them via `||` then to avoid code duplication. Recommendation: leave code as is but show this for one of the pairs as example, so user can better follow how this evolved... – Aconcagua Jan 16 '19 at 06:45
Yes @Aconcagua is right, you can combine the first 2 if statements to else if (((tolower(p1) == 'p') && (tolower(p2) == 'r')) || ((tolower(p2) == 'p') && (tolower(p1) == 'r'))) , and so on – gameon67 Jan 16 '19 at 07:07

score 1 · Answer 2 · edited Jun 20 '20 at 09:12

Out of the variables you have declared, you have initialized(user input) only p1 and p2; the rest P,p,R,r,S,s of them are still uninitialized and have garbage value.

Later in the program, you are comparing these with the p1 and p2. This is an undefined behavior. Therefore, initialize them before do the comparison operation on them.

Just for clarification; P,p,R,r,S,s are called identifiers, and you need float value to assign them with(since they are of typefloat).

It's more likely that you need char instead of float.

char  p1, p2;
std::cin >>p1 >> p2;

for comparison, you should be doing

if (std::to_upper(p1) == 'P' && std::to_upper(p2) == 'R')
    ^^^^^^^^^^^^^^       ^^^^   ^^^^^^^^^^^^^^       ^^^^

Note that, std::toupper is for to covert the character into uppercase, which will reduce the number of checks what you made.

Although you need to check whether p1 and p2 are any of 'P' or 'R' or 'S', if they are equal.

Using ternary operator, one can write a single liner code as follows:

#include <iostream>
#include <cctype>   // std::toupper

int main()
{
    std::cout << "Input Player 1 and Player 2 Choices\n";
    char p1, p2; std::cin >> p1 >> p2;

    p1 = std::toupper(p1); // convert to upper case
    p2 = std::toupper(p2); // convert to upper case

    std::cout << (
        (p1 == p2 && (p1 == 'P' || p1 == 'R' || p1 == 'S')) ? "tie\n" 
        : (p1 == 'P' && p2 == 'R') || (p1 == 'R' && p2 == 'P') ? "Paper covers rock!\n"
        : (p1 == 'S' && p2 == 'R') || (p1 == 'R' && p2 == 'S') ? "Rock breaks scissors!\n"
        : (p1 == 'S' && p2 == 'P') || (p1 == 'P' && p2 == 'S') ? "Scissors cut paper!\n"
        : "Wrong input!\n"
        );

    return 0;
}

score 0 · Answer 3 · edited Jun 20 '20 at 09:12

This very much reads like a class assignment, so I will describe the errors rather than simply presenting working code.

Your first error is using floats at all: floating point types (half, float, double, etc.) are for (approximating) real numbers. The appropriate choice here is char, a basic "character" type, but any integer type should work as char is also the (usually) smallest integer type.

Your second error is attempting to use variable names (properly: identifiers) as character literals:

float P; // uninitialized floating point object
'P' // literal character "P"

Your third error, a logic error, is making all comparisons in the same order with the same variables:

// edited to correct previous error
if ((p1 == 'P') && (p2 == 'R')) // checks if p1 wins
 cout<<"Paper covers rock.";
if ((p1 == 'p') && (p2 == 'r')) // checks if p1 wins
 cout<<"Paper covers rock.";
if ((p1 == 'P') && (p2 == 'r')) // checks if p1 wins
 cout<<"Paper covers rock.";
if ((p1 == 'p') && (p2 == 'R')) // checks if p1 wins - shouldn't we check for p2 winning somewhere?
 cout<<"Paper covers rock.";

Your fourth error, also a logic error, based on the assignment constraints, is not printing which player wins. As previously mentioned, your code can currently only detect either a win by player 1 or a tie.

Your fifth, debatable, "error" is not printing a linebreak (append \n to string, or << std::endl to std::cout) before exiting.

I am getting all of my outputs instead of just one

3 Answers3

This very much reads like a class assignment, so I will describe the errors rather than simply presenting working code.