rock paper scissor

Question

I just started to learn C++ and I decided to make a rock paper scissor game. It works very well but I would like to know how could I improve it further. Thank you very much in advance for your help.

#include <iostream>
#include <string>
#include <time.h>

int main()
{   
    using namespace std;

    string playagain = "yes";
    string computerchoice;
    string playerChoice;



    // computer
    int computerNumber;

    //rounds + statistics

    string name;
    int playedRounds =0;
    int wonRounds = 0;
    int lostRounds = 0;
    int tieRounds = 0;

    // game text
    cout << "_______________________________________________" << endl;
    cout << "welcome it's a rock paper scissor game" << endl;
    cout << "Whats your name?" << endl;

    cin >> name;


    cout << "well hey " +name +" please Choose one "<< endl;
    cout << "rock - paper - scissor" << endl;
    cout << "_______________________________________________" << endl;

    while (playagain == "yes") {     

        // generating a random number   

        srand(static_cast<unsigned int>(time(NULL)));
        computerNumber = rand() % 3 + 1;

        // ________________________________ computer picking a element

        if (computerNumber == 1) {
            computerchoice = "rock";
        }
        else if (computerNumber == 2) {
            computerchoice = "paper";
        }
        else {
            computerchoice = "scissor";
        }

        //_________________________________

        // player choice + go to line

        playerChoice: {

        cin >> playerChoice;

        }

        if (playerChoice != "rock" && "paper" && "scissor") {

            cout << "" << endl;
            cout << "Please choose from one of them"<<endl;
            cout << "ROCK - PAPER - SCISSOR" << endl;
            cout << "" << endl;         
            goto playerChoice;
        }

        // player vs computer
        cout << playerChoice + " VS " + computerchoice << endl;

        // game rules

        if (playerChoice == "rock" && computerchoice == "paper") {
            cout << "you lost";  playedRounds++; lostRounds++;
        }
        else if (playerChoice == "rock" && computerchoice == "scissor") {
            cout << "you won";  playedRounds++; wonRounds++;
        }
        else if (playerChoice == "scissor" && computerchoice == "paper") {
            cout << "you won";  playedRounds++; wonRounds++;
        }
        else if (playerChoice == "scissor" && computerchoice == "rock") {
            cout << "you lost"; playedRounds++; lostRounds++;
        }
        else if (playerChoice == "paper" && computerchoice == "scissor") {
            cout << "you lost"; playedRounds++; lostRounds++;
        }
        else if (playerChoice == "paper" && computerchoice == "rock") {
            cout << "you won"; playedRounds++; wonRounds++;
        }
        else {
            cout << "tie"; playedRounds++; tieRounds++;
        }
        //__________________________________________________________________

            cout << "" << endl;
            cout << "" << endl;

            cout << "_______________________________________________" << endl;
            cout << "would u like to play again? or if u wanna check out your statistics u can just type  = sta" << endl;
            cout << "yes - no  - sta" << endl;
            cout << "_______________________________________________" << endl;
            cout << "" << endl;
            cout << "" << endl;

            playagain:{   // GO TO LINE     

            cin >> playagain;
            }

            if (playagain == "yes") {

            cout << "_______________________________________________" << endl;
            cout << "Choose wisely, rock - paper - scissor" << endl;
            cout << "rock - paper - scissor" << endl;
            cout << "_______________________________________________" << endl;
        }

            else if (playagain == "no") {
            cout << "have a nice day! now the program is going to shutdown";

        }

            else if (playagain == "sta") {
                cout << "" << endl;
                cout << "" << endl;

                cout << "Player name:" +name <<endl;
                cout << "Played Rounds: "; cout << +playedRounds << endl;
                cout << "Won Rounds: "; cout << +wonRounds << endl;
                cout << "lost Round: "; cout << +lostRounds << endl;
                cout << "Tie Rounds: "; cout << +tieRounds << endl;     
                cout << "would u like to continue ? " << endl;
                cout << "yes for continue no for exit " << endl;

                goto playagain;
            }

            else {

            cout << "i couldn't understand could u repeat please?" << endl;     
            goto playagain; 
        }           
    }
}

Answer 1

Welcome to Code Review and welcome to C++! Let's go through the code and see what can be improved.

In C++, the headers of the form <xxx.h> are deprecated, which means they should not be used. You are recommended to use

#include <ctime>

instead of #include <time.h>.

using namespace std; is considered bad practice because it causes name clashes. It will cause problems when you try to use common identifiers like size, count, find, etc. See Why is using namespace std; considered bad practice?. Explicitly qualify the names with std:: instead. You will find that this makes the code more readable when you deal with larger programs and multiple libraries.

I notice that your code structure looks like this:

#include <iostream>
#include <string>
#include <time.h>

int main()
{
    // ... sea of code ...
}

You have one big main function that does pretty much everything. Well, that's not good for readability. We will try to break it down to small parts. Each part should do one logical thing (e.g., ask for user input, randomly select rock/paper/scissor, etc.)

You represent paper/scissor/rock by the strings "paper", "scissor", and "rock". This is a bit wasteful. You can define an enumeration like this:

enum class Move {
    paper, scissor, rock
};

and define the game logic:

enum class Result {
    win, lose, tie
};

constexpr bool beats(Move a, Move b)
{
    return (static_cast<int>(a) + 2) % 3 == static_cast<int>(b);
}

constexpr Result compete(Move a, Move b)
{
    if (beats(a, b))
        return Result::win;
    else if (beats(b, a))
        return Result::lose;
    else
        return Result::tie;
}

The game has some states — the player name and the statistics. These can be wrapped in a class:

class RPS_game {
public:
    // std::string_view requires <string_view>
    // std::uint_fast32_t requires <cstdint>
    RPS_game(std::string_view name, std::uint_fast32_t seed)
        :player_name{name}, seed{seed}
    {
    }

    void run();

    // ...
private:
    std::string player_name;
    int total_rounds = 0;
    int won_rounds = 0;
    int lost_rounds = 0;
    int tie_rounds = 0;

    // requires <random>
    std::mt19937 seed;
};

Then, you can initialize the game and run it like this: (I omitted the greeting part for the sake of simplicity)

int main()
{
    std::string name;
    std::cout << "What is your name? ";
    getline(std::cin, name);

    auto seed = static_cast<std::uint_fast32_t>(std::random_device{}());
    RPS_game game{name, seed};

    game.run();
}

You may have noticed that I stored the seed as a std::mt19937 random number engine and use std::random_device to seed it, instead of using rand and time. In C++, std::rand is considered to be a low quality random number generator because the underlying algorithm is not specified and the low bits are often non-uniform. See Why is the new random library better than std::rand()?.

The following code generates the random move:

// generating a random number   

srand(static_cast<unsigned int>(time(NULL)));
computerNumber = rand() % 3 + 1;

// ________________________________ computer picking a element

if (computerNumber == 1) {
    computerchoice = "rock";
}
else if (computerNumber == 2) {
    computerchoice = "paper";
}
else {
    computerchoice = "scissor";
}

This should be made into its own function:

class RPS_game {
public:
    // ...
    Move generate_move();
    // ...
};

and you can implement it like this:

Move RPS_game::generate_move()
{
    std::uniform_int_distribution<int> dist(0, 2);
    return static_cast<Move>(dist(seed));
}

Now is the game logic. goto is not recommended because it makes the code harder to understand. See Edgar Dijkstra's Go To Statement Considered Harmful .

When I try to summarize the game logic in the simplest way possible, here's what I get for one round:

Result RPS_game::round()
{
    Move player_move = get_player_move();
    Move computer_move = generate_move();

    auto result = compete(player_move, computer_move);
    switch (result) {
    case Result::win:
        ++won_rounds;
        break;
    case Result::lose:
        ++lost_rounds;
        break;
    case Result::tie:
        ++tie_rounds;
        break;
    }
    ++total_rounds;
    return result;
}

And for the whole game:

void RPS_game::run()
{
    do {
        round();
        display_stats();
    } while (wanna_play_again());

    std::cout << "Goodbye!";
}

Following this approach, you can make your code more organized and more readable.

Answer 2

Welcome to the wonderful land of computer programming!

There are a few things you could do that could help clean up that code. However, if this is for a class, I would recommend not to add functions, make sure the player is giving a valid safe input, and avoiding namespace issues for future development.

But if it is for a class, here are yet a few

• Where can nested loops be used?
  • The flow control for playing again could be reworked as the "no" test isn't needed because the output could be outside the loop.
  • Any goto statements make it difficult to read because we typically can't easily distinguish the difference as we can we code blocks.
• Using proper vernacular and grammar would increase your max audience by roughly 5%.

Edit:
I wanted to add, that after 2^32 + 1 runs of your program, you'll see that the computer has a slight bias towards 0. This is pretty common when using making computer AI choices with most random number generators. This is because the of the way the modulus operator works. 10 % 3 will have have a distribution of 4, 3, and 3. I wouldn't worry about it too much for Rock, Paper, Scissors (it's basically a 1:40,000,000,000,000,000) but it's something you should be made aware of.