Simple Rock Paper Scissors in C++

Question

I have made a Rock Paper Scissors game. It works fine but I would like to know how can I improve it further.

#include <iostream>
#include <algorithm>
#include <string>
#include <random>
#include <array>
#include <map>
#include <limits>

namespace
{
    enum Winner { Tie, Player, Computer, WinnerCount };
    enum Items { Rock, Paper, Scissors, ItemsCount };

    template<typename T, std::size_t N>
    using Matrix = std::array<std::array<T, N>, N>;
}


std::string ask_guess(const std::array<std::string, Items::ItemsCount>& data)
{
    std::string result;
    auto comp = [&result](const std::string& str){ return str == result;};
    bool is_good = false;

    while (!is_good)
    {
        std::cout << "Enter your choice, rock, paper, or scissors: ";
        std::getline(std::cin, result);

        if (std::cin.fail() || std::none_of(data.begin(), data.end(), comp))
        {
            std::cout << "Not valid input.\n";
            if (!result.empty()) result.clear();
            is_good = false;
        }
        else
        {
            is_good = true;
        }

        std::cin.clear();
        std::cin.ignore(std::cin.rdbuf()->in_avail());
    };

    return result;
}

bool play_again()
{
    char result;
    bool is_good = false;

    while (!is_good)
    {
        std::cout << "Do you want to play again? [y/n]: ";
        std::cin >> result;
        result = std::toupper(result);

        if (std::cin.fail() || (result != 'Y' && result != 'N'))
        {
            std::cout << "Not valid input.\n";;
            is_good = false;
        }
        else
        {
            is_good = true;
        }

        std::cin.clear();
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    };

    return result == 'Y';
}

int main()
{
    // Set up game
    const Matrix<Winner, Winner::WinnerCount> table
    {
        {  // Rock      Paper      Scissors
            { Tie,      Computer,  Player   }, // Rock
            { Player,   Tie,       Computer }, // Paper
            { Computer, Player,    Tie      }, // Scissors
        }
    };

    const std::array<std::string, Items::ItemsCount> data
    {
        "rock",
        "paper",
        "scissors"
    };

    std::map<std::string, Items> bindingItems
    {
       { data[Items::Rock],      Items::Rock     },
       { data[Items::Paper],     Items::Paper    },
       { data[Items::Scissors],  Items::Scissors },
    };

    std::map<Winner, std::string> winnerResult
    {
       { Winner::Tie,        "It's a tie!\n\n"  },
       { Winner::Player,     "You win!\n\n"     },
       { Winner::Computer,   "You lose!\n\n"    },
    };

    std::mt19937 engine{ std::random_device()() };
    auto dist = std::uniform_int_distribution<>(0, Items::ItemsCount - 1);

    // Game loop
    do
    {
        std::cout << "\n\n\tRock Paper Scissors Game\n\n";

        const auto userChoice = ask_guess(data);
        const Items player = bindingItems[userChoice];

        const Items computer = static_cast<Items>(dist(engine));
        const auto computerChoice = data[computer];

        std::cout << "\n\nYou entered: " << userChoice << '\n';
        std::cout << "Computer chose: " << computerChoice << '\n';

        std::cout << winnerResult[table[player][computer]];
    }
    while(play_again());
}

Answer 1

Here are some things that may help you improve your code.

Simplify the code

This seems to be rather a lot of lines of code for a very simple game. For example, the ask_guess and play_again both do essentially the same thing. They show a prompt, wait for input and then check to assure the input is one of the valid choices. Since it's done for two different things and has both associated data and functionality, that suggests an object.

Simplify winner detection

Each player has only three choices, and if we take them in the order of "rock, paper, scissors" then for any choice n, the choice n+1 wins, choice n-1 loses, and n means a tie (obviously this is all mod 3). This allows for very simple mathematics to determine the winner.

Make all strings named constants

If you ever wanted to make, say, a German version of the game, it would be simple if all of the strings were in one location. If they're all const, then it doesn't have much impact on the runtime speed and there really aren't that many strings.

A proposed alternative

Here's an alternative that incorporates all of these suggestions:

#include <iostream>
#include <string>
#include <vector>
#include <algorithm>
#include <random>
#include <cctype>

class Menu {
private:
    const std::string prompt; 
    const std::vector<std::string> choices;
    const std::string error_msg; 
public:
    Menu(const std::string &prompt_string, const std::vector<std::string>& items, const std::string &err_msg)
    : prompt{prompt_string},
      choices{items},
      error_msg{err_msg}
    {}
    size_t size() const { return choices.size(); }
    const std::string& operator[](int i) const { return choices[i]; }
    size_t get_choice(std::istream& in=std::cin, std::ostream& out=std::cout) const {
        bool first = true;
        auto it = choices.end();
        do {
            if (!first) {
                out << error_msg;
            }
            out << prompt;
            std::string result;
            std::getline(in, result);
            std::transform(result.begin(), result.end(), result.begin(), [](unsigned char ch){ return std::tolower(ch);});
            it = std::find(choices.begin(), choices.end(), result);
            first = false;
            in.clear();
            in.ignore(in.rdbuf()->in_avail());
        } while (it == choices.end());
        return std::distance(choices.begin(), it);
    }
};

int main() {
    const Menu rps{"Enter your choice, rock, paper, or scissors: ", 
        { "rock", "paper", "scissors"}, "Not valid input.\n"};
    const Menu again{"Do you want to play again? [y/n]: ", 
        {"y", "n"}, "Not valid input.\n"};
    const std::vector<std::string> results{
        "It's a tie!", "You win!", "You lose!"};
    const std::string title{"\n\n\tRock Paper Scissors Game\n\n"};
    const std::string youchose{"\n\nYou entered: "};
    const std::string computerchose{"\nComputer chose: "};
    std::mt19937 engine{ std::random_device()() };
    auto dist = std::uniform_int_distribution<>(0, rps.size()-1);
     // Game loop
    do {
        std::cout << title;
        const auto user = rps.get_choice();
        const auto computer = dist(engine);
        std::cout << youchose << rps[user] 
            << computerchose << rps[computer] << '\n'
            << results[(user + rps.size() - computer) % rps.size()] 
            << "\n\n";
    }
    while(!again.get_choice());
}

Answer 2

Simplification on handling the is_good variable

You can assign is_good directly to the boolean expression:

is_good = ! (std::cin.fail() || std::none_of(data.begin(), data.end(), comp));

And use it to decide your successive actions:

    if (!is_good)
    {
        std::cout << "Not valid input.\n";
        if (!result.empty()) result.clear();
    }

As is_good is too vague I suggest is_input_valid to be more precise.

Always at least once -> do while

In both ask_guess and play_again the loop should run at least once, so I suggest using do while to simplify.