How to fix a function that only executes every other time it is called in C++?

Question

I am trying to learn basic C++ this summer and I am struggling with this Rock, Paper, Scissors game program.

For some reason, when I run the program, it only works every other time the do-while loop iterates. So the first time the program executes correctly, then the second time the menu choices print but when the user answers, it just displays the menu again. Then it works on the 3rd iteration, but not the 4th time, and so on.

So the getUserChoice function seems to get called and executes each time, but maybe not the determineWinner function?

I have tried a lot of different changes but I cannot seem to find the bug. Any help would be greatly appreciated!

int getComputerChoice(int computerChoice)
{
    //declare vars for min and max of random number
    const int MIN = 1;
    const int MAX = 3;
    
    //get system time
    unsigned seed = time(0);

    //randomize rand
    srand(seed);

    // Generate random number
    computerChoice = MIN + rand() % MAX;
    
    return computerChoice;
}

int getUserChoice(int userChoice)
{   
    //declare constants for menu choices
    const int ROCK = 1, PAPER = 2, SCISSORS = 3, QUIT = 4;
            
    cout<<"Rock, Paper, Scissors Game\n"
        <<"---------\n"
        <<"1) Rock\n"
        <<"2) Paper\n"
        <<"3) Scissors\n"
        <<"4) Quit\n\n"
        <<"Enter your choice:\n";
    cin>>userChoice;
    
    //validate input
    while (userChoice <1 || userChoice>4)
    {
        cout<<"Invalid selection. Enter 1, 2, 3, or 4:\n";
        cin>>userChoice;
    }
    
    return userChoice;      
}

void determineWinner(int userChoice, int computerChoice)
{
    
    if(userChoice == 1 && computerChoice == 2)
    {
        cout<<"\nYou selected: ROCK\n\n"
            <<"The computer selected: PAPER\n\n"
            <<"Computer wins! Paper wraps rock!\n\n" 
            <<"*********************************\n"<<endl;
    }
    else if(userChoice == 1 && computerChoice == 3)
    {
        cout<<"\nYou selected: ROCK\n\n"
            <<"The computer selected: SCISSORS\n\n"
            <<"You win! Rock smashes scissors!\n\n"
            <<"*********************************\n"<<endl;
    }
    else if(userChoice == 2 && computerChoice == 1)
    {
        cout<<"\nYou selected: PAPER\n\n"
            <<"The computer selected: ROCK\n\n"
            <<"You win! Paper wraps rock!\n\n"
            <<"*********************************\n"<<endl;  
    }
    else if(userChoice == 2 && computerChoice == 3)
    {
        cout<<"\nYou selected: PAPER\n\n"
            <<"The computer selected: SCISSORS\n\n"
            <<"Computer wins! Scissors cut paper!\n\n"
            <<"*********************************\n"<<endl;
    }
    else if(userChoice == 3 && computerChoice == 1)
    {
        cout<<"\nYou selected: SCISSORS\n\n"
            <<"The computer selected: ROCK\n\n"
            <<"Computer wins! Rock smashes scissors!\n\n"
            <<"*********************************\n"<<endl;
    }
    else if(userChoice == 3 && computerChoice == 2)
    {
        cout<<"\nYou selected: SCISSORS\n\n"
            <<"The computer selected: PAPER\n\n"
            <<"You win! Scissors cut paper!\n\n"
            <<"*********************************\n"<<endl;
    }
    else
        cout<<"\nTie, No winner!\n\n"
            <<"*********************************\n"<<endl;
    
}   

int main()
{   
    //declare vars
    int userChoice, computerChoice;

    do
    {
        //call determineWinner function
        determineWinner(getUserChoice(userChoice), getComputerChoice(computerChoice));
    }
    while (getUserChoice(userChoice) != 4);
    
    system ("pause");
    return 0;
}

Why do your `getUserChoice` and `getComputerChoice` functions take an argument? — Nathan Pierson, Jul 28 '21 at 15:52
Your `do-while` loop calls `getUserChoice` once when invoking `determineWinner` and a second time when evaluating the loop condition. Both times it will prompt for user input. If you only want to prompt once, call `getUserChoice` once and save the result to a variable. — Nathan Pierson, Jul 28 '21 at 15:53
Also, consider usage of `enum` for your constants instead of just using magic numbers, as it will make your code much cleaner and the detection of bugs faster. — mathway, Jul 28 '21 at 15:58
May help solve a future bug: [srand() — why call it only once?](https://stackoverflow.com/questions/7343833/srand-why-call-it-only-once) — user4581301, Jul 28 '21 at 16:24

score 0 · Answer 1 · answered Jul 28 '21 at 16:03

0

getUserChoice is getting called twice per while loop: once inside the body and once in the condition.

do
{
        //call determineWinner function
        determineWinner(
   getUserChoice(userChoice), // Called once here... 
   getComputerChoice(computerChoice));
}
while (getUserChoice(userChoice) // And again here. 
    != 4);

You might consider saving the return value to a variable to you can reuse it without prompting the user again:

do
{
  userChoice = getUserChoice(userChoice);
        //call determineWinner function
        determineWinner(userChoice, getComputerChoice(computerChoice));
}
while (userChoice != 4);

answered Jul 28 '21 at 16:03

TheMikeste1

87
2
6

Not the downvoter, but I'd flip this around a bit and have `determineWinner` get the choices and return whether or not to continue. That turns the code into a really simple `while (determineWinner()) {}` – user4581301 Jul 28 '21 at 16:23
Thanks so much, everyone! I got it fixed and it is working beautifully! – user16547206 Jul 28 '21 at 17:42

How to fix a function that only executes every other time it is called in C++?

1 Answers1