18
\$\begingroup\$

Here's my take at the Rock-Paper-Scissors-Lizard-Spock challenge. The outcomes are as follows:

  • Scissors cuts paper
  • paper covers rock
  • rock crushes lizard
  • lizard poisons Spock
  • Spock smashes scissors
  • scissors decapitate lizard
  • lizard eats paper
  • paper disproves Spock
  • Spock vaporizes rock
  • rock crushes scissors

I've decided to implement this in procedural form, partly because it's just one player against another. The program doesn't output any of the above outcomes, just generic ones, but I may try to figure out the former at another time. Points are awarded to the winner each turn with the total displayed upon termination. I've also used std::rand() instead of something "better" (such as std::mt19937 and std::uniform_int_distribution) since this is a simple program.

My main concerns:

  • I feel that there can be a good substitute to possibleMoves; it looks quite large.
  • I couldn't find a good STL function to do the searching in determineOutcome(), although what I have now may suffice.
  • Not enough/not good enough validation to avoid breaking the game.

Other than that, nothing else sticks out to me. Please do criticize anything you may find.

#include <array>
#include <cstdlib>
#include <ctime>
#include <iostream>
#include <stdexcept>
#include <string>
#include <utility>

enum Weapon { ROCK=1, PAPER, SCISSORS, LIZARD, SPOCK };
enum Outcome { WIN, LOSE, DRAW };

typedef std::pair<Weapon, Weapon> PlayerMoves;

const std::array<PlayerMoves, 10> possibleMoves =
    { std::make_pair(SCISSORS, PAPER),
      std::make_pair(PAPER, ROCK),
      std::make_pair(ROCK, LIZARD),
      std::make_pair(LIZARD, SPOCK),
      std::make_pair(SPOCK, SCISSORS),
      std::make_pair(SCISSORS, LIZARD),
      std::make_pair(LIZARD, PAPER),
      std::make_pair(PAPER, SPOCK),
      std::make_pair(SPOCK, ROCK),
      std::make_pair(ROCK, SCISSORS)
    };

std::istream& operator>>(std::istream& in, Weapon& weapon)
{
    int val;

    if (in >> val)
        weapon = static_cast<Weapon>(val);
    else
        throw std::logic_error("invalid weapon");

    return in;
}

std::ostream& operator<<(std::ostream& out, Weapon const& weapon)
{
    switch (weapon)
    {
        case ROCK:     return out << "rock";
        case PAPER:    return out << "paper";
        case SCISSORS: return out << "scissors";
        case LIZARD:   return out << "lizard";
        case SPOCK:    return out << "Spock";
        default: throw std::logic_error("invalid weapon");
    }
}

Weapon getPlayerWeapon()
{
    std::cout << "\n\n(1) -> Rock\n";
    std::cout << "(2) -> Paper\n";
    std::cout << "(3) -> Scissors\n";
    std::cout << "(4) -> Lizard\n";
    std::cout << "(5) -> Spock\n\n";

    Weapon weapon;

    do
    {
        std::cout << "Your move: ";
        std::cin >> weapon;
    } while (weapon < ROCK || weapon > SPOCK);

    return weapon;
}

Weapon getOpponentWeapon()
{
    return static_cast<Weapon>(1 + std::rand() % 5);
}

Outcome determineOutcome(PlayerMoves const& playerMoves)
{
    if (playerMoves.first == playerMoves.second)
        return DRAW;

    for (auto iter = possibleMoves.cbegin(); iter != possibleMoves.cend(); ++iter)
    {
        if (playerMoves == *iter)
            return WIN;
    }

    return LOSE;
}

int main()
{
    std::srand(static_cast<unsigned int>(std::time(nullptr)));

    std::cout << "Rock, Paper, Scissors, Lizard, Spock\n";

    unsigned int playerScore = 0;
    unsigned int opponentScore = 0;

    for (;;)
    {
        Weapon playerWeapon = getPlayerWeapon();
        Weapon opponentWeapon = getOpponentWeapon();
        PlayerMoves playerMoves = std::make_pair(playerWeapon, opponentWeapon);

        std::cout << "\n\nYou've picked  : " << playerWeapon;
        std::cout << "\nComputer picked: " << opponentWeapon;

        Outcome outcome = determineOutcome(playerMoves);

        switch (outcome)
        {
            case WIN:
                std::cout << "\n\nYou win!  Point awarded to you.";
                playerScore++;
                break;
            case LOSE:
                std::cout << "\n\nYou lose!  Point awarded to opponent.";
                opponentScore++;
                break;
            case DRAW:
                std::cout << "\n\nIt's a draw!  No points awarded.";
                break;
            default:
                throw std::logic_error("\n\ninvalid outcome");
                break;
        }

        std::cout << "\n\nPlay again (Y/N)? ";
        char choice;
        std::cin >> choice;

        if (choice == 'n' || choice == 'N') break;
    }

    std::cout << "\n\nFinal Scores:\n\n";
    std::cout << "Player  : " << playerScore;
    std::cout << "\nOpponent: " << opponentScore;
}
\$\endgroup\$
8
  • \$\begingroup\$ If you reordered your enum to { Rock, Spock, Paper, Lizard, Scissors } you could use modular arithmetic to determine the winner. \$\endgroup\$ Commented Dec 1, 2013 at 16:33
  • \$\begingroup\$ @MichaelUrman, how do you figure that? \$\endgroup\$
    – Malachi
    Commented Dec 1, 2013 at 19:33
  • 6
    \$\begingroup\$ @Malachi, look at the winning combos. For clarity, repeat it out: Rock, Spock, Paper, Lizard, Scissors, Rock, Spock. With that ordering, each item loses to the next two in the list (or wins against the two right before it). So for example bool defeats(Weapon2& a, Weapon2& b) { return ((5 + a - b) % 5) < 3; }. \$\endgroup\$ Commented Dec 2, 2013 at 1:22
  • \$\begingroup\$ I see what you mean. @MichaelUrman that is very interesting. I will have to look into that \$\endgroup\$
    – Malachi
    Commented Dec 2, 2013 at 2:29
  • \$\begingroup\$ @MichaelUrman Clever, but I don't think it's a winner in terms of extensibility. Picture updating the code to Rock-Paper-Scissors-Lizard-Spock-Batman-SpiderMan-Wizard-Glock.. still appealing? \$\endgroup\$ Commented Dec 2, 2013 at 13:39

2 Answers 2

12
\$\begingroup\$

The array does not seem like the logical choice for container.
You are going to spend most of the time looking things up. This suggests that you need some form of map. Because of its size I would just use std::map. But you only have the true values stored so we only need to check for existence so we can use std::set instead.

const std::set<PlayerMove> possibleMoves =
    { std::make_pair(SCISSORS, PAPER),
      std::make_pair(PAPER, ROCK),
      std::make_pair(ROCK, LIZARD),
      std::make_pair(LIZARD, SPOCK),
      std::make_pair(SPOCK, SCISSORS),
      std::make_pair(SCISSORS, LIZARD),
      std::make_pair(LIZARD, PAPER),
      std::make_pair(PAPER, SPOCK),
      std::make_pair(SPOCK, ROCK),
      std::make_pair(ROCK, SCISSORS)
    };

Your test is now simplified too:

Outcome determineOutcome(PlayerMoves const& playerMoves)
{
    if (playerMoves.first == playerMoves.second)
        return DRAW;

    return possibleMoves.find(playerMoves) == possibleMoves.end()
                  ?  LOSE
                  :  WIN;
}

I have a template to handle enum (so I don't need to re-write the same code all the time).

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

namespace Serializer
{
    template<typename T>
    struct SerializeTraits
    {};

    template<typename T>
    struct EnumSerializer
    {
        T value;
        EnumSerializer(T const& v) : value(v) {}
    };
    template<typename T>
    struct EnumDeSerializer
    {
        T& value;
        EnumDeSerializer(T& v) : value(v) {}
    };

    template<typename T>
    EnumSerializer<T> makeEnumSerializer(T const& v) {return EnumSerializer<T>(v);}
    template<typename T>
    EnumDeSerializer<T> makeEnumDeSerializer(T& v) {return EnumDeSerializer<T>(v);}

    template<typename T>
    std::ostream& operator<<(std::ostream& stream, EnumSerializer<T> const& val)
    {
        return stream << SerializeTraits<T>::names[static_cast<int>(val.value)];
    }
    template<typename T>
    std::istream& operator>>(std::istream& stream, EnumDeSerializer<T> const& val)
    {
        typedef std::vector<std::string>::const_iterator    Iter;

        std::string value;
        stream >> value;

        Iter find   = std::find(std::begin(SerializeTraits<T>::names), std::end(SerializeTraits<T>::names), value);
        if (find == std::end(SerializeTraits<T>::names))
        { 
            stream.setstate(std::ios::failbit);
        }
        else
        {
            val.value   = static_cast<T>(std::distance(std::begin(SerializeTraits<T>::names), find));
        }

        return stream;
    }
}

To use it you just need to declare a SerializeTraits<> for the enum type. In your case it looks like this:

enum Weapon {ROCK=1, PAPER, SCISSORS, LIZARD, SPOCK};
namespace  Serializer
{
    template<>
    struct SerializeTraits<Weapon>
    {
        static const std::vector<std::string>    names;
    };
}
const std::vector<std::string>   Serializer::SerializeTraits<Weapon>::names = {"", "ROCK", "PAPER", "SCISSORS", "LIZARD", "SPOCK"};

Then reading and writting enum's become the same everywhere (and its handeled for you).

int main()
{
    using namespace Serializer;
    std::cout << makeEnumSerializer(ROCK);

    Weapon  x;
    std::cin >> makeEnumDeSerializer(x);

    std::cout << makeEnumSerializer(x);
}

Also has the advantage that as you update the Weapons (and the associated name string) you do not need to modify code it will always stay in sync.

Answers to comments:

Unfortunately, I cannot std::set or std::vector with initialization I still don't have full access to C++11.

Alternative to Initialization list

class MySetWithInit: public std::list
{
    public: MySetWithInit()
    {
      insert(std::make_pair(SCISSORS, PAPER));
      insert(std::make_pair(PAPER, ROCK));
      insert(std::make_pair(ROCK, LIZARD));
      insert(std::make_pair(LIZARD, SPOCK));
      insert(std::make_pair(SPOCK, SCISSORS));
      insert(std::make_pair(SCISSORS, LIZARD));
      insert(std::make_pair(LIZARD, PAPER));
      insert(std::make_pair(PAPER, SPOCK));
      insert(std::make_pair(SPOCK, ROCK));
      insert(std::make_pair(ROCK, SCISSORS));
    }
} possibleMoves; // Note variable declaration here.
                 // This indicates type is being declared for this usage
                 // only.

Note: normally you should not inherit from standard containers. This is a case where it is OK as there are no plans to re-use the type and it is not being deleted via a pointer to the base class. This should be placed in *.cpp file not a header.

Until then, I'll either have to stay with std::array or just retrieve the set from a function (which I know is not efficient).

Is initialization from a function less effecient.

No. Not really. All modern C++ compilers have RVO and NRVO optimization as standard. This means if a function returns by value the compiler will more than likely elide the copy of the value out of the function, ie it effectively builds the value in place and no intermediate copies are used (this works through multiple levels of copying).

I don't quite know what all this does, but I'll still try to learn.

Good.

But really there are just a couple of common tricks.

  1. Template traits class.
    The templatized version holds nothing (see SerializeTraits). But the code depends on particular values being there. So if you try and use it then compilation will fail. Thus to use the code you need to actually declare a template specialization explicitly for your class (see SerializeTraits<Weapon>).

  2. make_X function
    template functions can deduce their type based on the parmaeters passed to the function. While a class can not. So you can use template functions to create objects of the correct type without having to be explicit about it.

As for the Serializer code, I assume this should go into a header and included

Yes just put the serialize code in a header file.

It will then work for all enum types you want.

Now I just have to put in good input validation

Validation code with custom stream inputter.

When you write the stream input operator operator>> you should validate input and set the streams failbit as appropriate (I changed the code from yesterday to do this).

Now detecting bad input and correcting for it is the same as any other type.

std::cout << "What Weapon do you want\n";

Weapon  weapon;
std::cin >> makeEnumDeSerializer(weapon);
while(std::cin.fail())
{
    std::cout << "That is not a valid Weapon\n";
    std::cin.clear();
    std::cin >> makeEnumDeSerializer(weapon);
}
\$\endgroup\$
5
  • \$\begingroup\$ Thanks. I don't quite know what all this does, but I'll still try to learn. Unfortunately, I cannot std::set or std::vector with initialization I still don't have full access to C++11. Until then, I'll either have to stay with std::array or just retrieve the set from a function (which I know is not efficient). As for the Serializer code, I assume this should go into a header and included. \$\endgroup\$
    – Jamal
    Commented Dec 1, 2013 at 5:58
  • \$\begingroup\$ There is an alternative for C++03 initialization (that initialization list replaces (because it is neater and shorter)). \$\endgroup\$ Commented Dec 1, 2013 at 8:36
  • \$\begingroup\$ Okay, I have it working. It took me a while to realize that I'm now supposed to input the actual moves instead of corresponding numbers, which is nice. Now I just have to put in good input validation so that I can prevent the game from crashing. I may post a follow-up question sometime. \$\endgroup\$
    – Jamal
    Commented Dec 1, 2013 at 18:03
  • \$\begingroup\$ @Jamal: Another update on input. \$\endgroup\$ Commented Dec 1, 2013 at 18:30
  • \$\begingroup\$ Okay, now the input validation works! I just now caught your note about the change in the earlier code. That also explains why I kept getting an exception. I'll surely hold onto this Serializer for other uses. \$\endgroup\$
    – Jamal
    Commented Dec 2, 2013 at 5:43
5
\$\begingroup\$

A few minor things:

  1. If you add a new weapon you have to remember to adjust the input loop

    while (weapon < ROCK || weapon > SPOCK)
    

    A solution to this problem is to have a LAST enum as well and new weapons should be inserted before that. Then the loop is

    while (weapon < ROCK || weapon >= LAST)
    
  2. I wouldn't call it possibleMoves. (ROCK, ROCK) is a possible move but it's not in the list. winningMoves would be more appropriate.

  3. You should be able to use std::find to find a winning move:

    if (std::find(possibleMoves.cbegin(), possibleMoves.cend(), playerMoves) != possibleMoves.cend())
        return WIN;
    
\$\endgroup\$
1
  • \$\begingroup\$ Ah, so that's how std::find should be used here. I was trying it myself, but I couldn't get it to work. The naming and enum are good points, too. It does look like a good idea to work with different weapons. \$\endgroup\$
    – Jamal
    Commented Dec 1, 2013 at 1:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.