12
\$\begingroup\$

Rock Paper Lizard Spock Monkey Banana

My original post
Based off of Malachi's post

I've changed the way rules work in this version to allow ties to result in double loss or double win. While statistically picking monkey wins against the computer, my intention is to make this a multi-player game which would require more strategy than the completely flat RPS or RPSLS.

I'm looking for any criticism/advice/review on the following code. I think the Rules structure is pretty complex and daunting. 

Dictionary<Gesture, Tuple<         // A dictionary of all the gestures
    Dictionary<Gesture, string>,   // A sub-dictionary of all of the gestures this one beats, and a string of why.
    Dictionary<Gesture, string>    // A sub-dictionary of all of the gestures this one loses to, and a string of why.
>>

I've considered moving it to a separate class. However, it does work perfectly fine as is, so I haven't.

Also you should note that the rules dictionary does contain a reason for why gestures lose, this isn't used in the current implementation, but I do plan to have it used when I have people playing in multi-player to be notified of "why they lost"... which is only slightly different from "why the other guy won".

Here is the full, runnable dump

class Program
{
    static void Main(string[] args)
    {
        var gameMenu = new string[] { "Play", "Clear Score", "Quit" };
        var me = new Human();
        var computer = new Computer();
        var playAgain = true;
        do
        {
            Utils.WriteLineColored("Options:", ConsoleColor.White);
            Utils.PrintMenu(gameMenu.ToList());
            switch (Utils.PromptForRangedInt(0, gameMenu.Length - 1, "Choose an Option: "))
            {
                case 0:
                    Console.Clear();
                    Game.Play(me, computer);
                    Console.WriteLine("Your scorecard: " + me.GetScoreCard() + Environment.NewLine);
                    break;
                case 1:
                    Console.Clear();
                    me.ClearScore();
                    Utils.WriteLineColored("Your score has been cleared", ConsoleColor.Green);
                    break;
                case 2:
                    Console.Clear();
                    playAgain = false;
                    Console.Write("Good bye, thanks for playing!\nPress any Key to contine...");
                    Console.ReadKey(true);
                    break;
            }
        } while (playAgain);
    }
}

enum Gesture
{
    Rock,
    Paper,
    Scissors,
    Lizard,
    Spock,
    Monkey,
    Banana
}

enum Performance
{
    Lost,
    Tied,
    Won
}

abstract class Player
{
    public uint Wins { get; private set; }
    public uint Loses { get; private set; }
    public uint Ties { get; private set; }

    public abstract Gesture GetMove();

    public string GetScoreCard()
    {
        return "[Wins: " + Wins + "] [Loses " + Loses + "] [Ties " + Ties + "]";
    }

    public void ClearScore()
    {
        Wins = Loses = Ties = 0;
    }

    public void GiveResult(Performance performance)
    {
        switch (performance)
        {
            case Performance.Lost: Loses++; break;
            case Performance.Tied: Ties++; break;
            case Performance.Won: Wins++; break;
        }
    }
}

class Human : Player
{
    public override Gesture GetMove()
    {
        Utils.PrintMenu(Game.Gestures.Select(g => g.ToString()).ToList());
        return Game.Gestures[Utils.PromptForRangedInt(0, Game.Gestures.Length - 1, "Please choose your Gesture: ")];
    }
}

class Computer : Player
{
    public override Gesture GetMove()
    {
        return (Gesture)Game.Gestures.GetValue(new Random().Next(Game.Gestures.Length));
    }
}

static class Game
{
    public static Gesture[] Gestures = (Gesture[])Enum.GetValues(typeof(Gesture));

    private static Dictionary<Gesture, Tuple<Dictionary<Gesture, string>, Dictionary<Gesture, string>>> Rules = new Dictionary<Gesture, Tuple<Dictionary<Gesture, string>, Dictionary<Gesture, string>>>()
    {
        {Gesture.Rock, Tuple.Create(new Dictionary<Gesture, string>(){
            {Gesture.Scissors,"Crushes"},
            {Gesture.Lizard,"Crushes"},
            {Gesture.Banana,"Smushes"}
        }, new Dictionary<Gesture, string>(){
            {Gesture.Paper,"Gets Covered By"},
            {Gesture.Spock,"Gets Vaporized By"},
            {Gesture.Monkey,"Gets Thrown By"},
        })},

        {Gesture.Paper, Tuple.Create(new Dictionary<Gesture, string>(){
            {Gesture.Rock,"Covers"},
            {Gesture.Spock,"Disproves"},
            {Gesture.Banana,"Slowly Suffocates"}
        }, new Dictionary<Gesture, string>(){
            {Gesture.Scissors,"Gets Cut By"},
            {Gesture.Lizard,"Gets Eaten By"},
            {Gesture.Monkey,"Gets Shredded By"},
        })},

        {Gesture.Scissors, Tuple.Create(new Dictionary<Gesture, string>(){
            {Gesture.Paper,"Cut"},
            {Gesture.Lizard,"Decapitates"},
            {Gesture.Banana,"Cuts"}
        }, new Dictionary<Gesture, string>(){
            {Gesture.Rock,"Gets Smashed By" },
            {Gesture.Spock,"Gets Crushed By"},
            {Gesture.Monkey,"Gets Broken By"},
        })},

        {Gesture.Lizard, Tuple.Create(new Dictionary<Gesture, string>(){
            {Gesture.Paper,"Eats"},
            {Gesture.Spock,"Poisons"},
            {Gesture.Banana,"Eats"}
        }, new Dictionary<Gesture, string>(){
            {Gesture.Rock,"Gets Crushed By"},
            {Gesture.Scissors,"Gets Decapitated By"},
            {Gesture.Monkey,"Gets Eaten By"},
        })},

        {Gesture.Spock, Tuple.Create(new Dictionary<Gesture, string>(){
            {Gesture.Rock,"Vaporizes"},
            {Gesture.Scissors,"Smashes"},
            {Gesture.Banana,"Studies"}
        }, new Dictionary<Gesture, string>(){
            {Gesture.Paper,"Gets Disproved By"},
            {Gesture.Lizard,"Gets Poisoned By"},
            {Gesture.Monkey,"Gets Boggled By" },
        })},

        {Gesture.Monkey, Tuple.Create(new Dictionary<Gesture, string>(){
            {Gesture.Rock,"Throws"},
            {Gesture.Paper,"Shreds"},
            {Gesture.Scissors,"Breaks"},
            {Gesture.Lizard,"Steps On"},
            {Gesture.Spock,"Boggles"},
        }, new Dictionary<Gesture, string>(){
            {Gesture.Monkey,"Gets Beaten By"},
            {Gesture.Banana,"Goes cRaZy From Eating The"}
        })},

        {Gesture.Banana, Tuple.Create(new Dictionary<Gesture, string>(){
            {Gesture.Monkey,"Crazes"},
            {Gesture.Banana,"Chills With"}
        }, new Dictionary<Gesture, string>(){
            {Gesture.Rock,"Gets Squished By"},
            {Gesture.Paper,"Gets Slowly Suffocated By"},
            {Gesture.Scissors,"Gets Cut By"},
            {Gesture.Lizard,"Gets Eaten By"},
            {Gesture.Spock,"Gets Beaten By"}
        })}
    };

    public static void Play(Player player1, Player player2)
    {
        Gesture p1move = player1.GetMove();
        Gesture p2move = player2.GetMove();

        Console.Write("Player 1 Chose ");
        Utils.WriteLineColored(p1move.ToString(), ConsoleColor.Green);
        Console.Write("Player 2 Chose ");
        Utils.WriteLineColored(p2move.ToString(), ConsoleColor.Green);

        Performance p1perf = WhatHappensToMe(p1move, p2move);
        Performance p2perf = WhatHappensToMe(p2move, p1move);

        player1.GiveResult(p1perf);
        player2.GiveResult(p2perf);

        //Report to the console what has happened to player 1
        if (p1perf == Performance.Tied)
            Console.WriteLine("It was a tie!");
        else
            Console.WriteLine("Player 1 {0} Because, {1}.", p1perf, GetReason(p1move, p2move, p1perf));
    }

    private static Performance WhatHappensToMe(Gesture myMove, Gesture theirMove)
    {
        return
            Rules[myMove].Item1.ContainsKey(theirMove) ? Performance.Won :
            Rules[myMove].Item2.ContainsKey(theirMove) ? Performance.Lost :
            Performance.Tied;
    }

    private static string GetReason(Gesture myMove, Gesture theirMove, Performance performance)
    {
        return myMove.ToString() + ' ' +
            (performance == Performance.Won ? Rules[myMove].Item1 : Rules[myMove].Item2)[theirMove]
            + ' ' + theirMove.ToString();
    }
}

static class Utils
{
    public static int PromptForRangedInt(int min = int.MinValue, int max = int.MaxValue, string prompt = "Please enter an Integer: ")
    {
        int g;
        do
        {
            Console.Write(prompt);
            if (int.TryParse(Console.ReadLine(), out g))
            {
                if (g >= min && g <= max)
                    return g;
                Console.WriteLine("You entered {0}, but the input must be in the range of ({1} - {2}. Please try again...", g, min, max);
            }
            else
                Console.WriteLine("That is not a number. Please try again...");
        } while (true);
    }

    public static void PrintMenu(List<string> values, int baseIndex = 0)
    {
        values.ForEach(value => Console.WriteLine("{0}: {1}", baseIndex++, value));
    }

    public static void WriteLineColored(string text, ConsoleColor color)
    {
        var curr = Console.ForegroundColor;
        Console.ForegroundColor = color;
        Console.WriteLine(text);
        Console.ForegroundColor = curr;
    }
}
\$\endgroup\$
1
  • 5
    \$\begingroup\$ Who said Banana! Where? Where? \$\endgroup\$
    – rolfl
    Commented Mar 17, 2014 at 14:58

1 Answer 1

Reset to default
5
\$\begingroup\$

It might be simpler to define rules like this ...

List<Tuple<Gesture,Gesture,string,string>> Rules = new List<Tuple<Gesture,Gesture,string,string>> {
    Tuple.Create(Gesture.Rock,Gesture.Scissors,"Crushes","Get Crushed By"),
    ... etc ...
};

... because ...

  • It's simpler (less "daunting")
  • Easier to read (triplets of winning gesture, losing gesture, reason)
  • Easier to e.g. load from a flat configuration/resource file.
  • Less error-prone (you define everything twice, once for the winner and once for the loser; what you forget to define half the rule? where do you verify that you haven't forgotten?)

You could convert such a structure into a dictionary of dictionaries at load-time, e.g. if you wanted the improved performance of a dictionary instead of scanning a list.

The mapping between gameMenu options and integers isn't obvious unless you read the code; and the code is scattered (a gameMenu definition, a switch statement, Utils.PrintMenu, and Utils.PromptForRangedInt methods); perhaps instead have a UserInput class ...

static class UserInput
{
    internal enum Choice { Play, Reset, Quit };
    static Choice PromptUserAndGetChoice() { etc. }
}

... because then everything would be co-located in one class.

I like to put default: throw NotImplementedException(); at the end of switch statements.

Instead of ...

return "[Wins: " + Wins + "] [Loses " + Loses + "] [Ties " + Ties + "]";

... perhaps ...

return string.Format("[Wins: {0}] [Loses {1}] [Ties {2}]", Wins, Loses, Ties);

... because I find it easier to read the format of the latter output string.

Instead of abstract Player with two subclasses, you could have a concrete player class which takes the abstraction as a delegate parameter passed into its constructor

Func<Gesture> GetMove;
Player(Func<Gesture> getMove)
{
    this.GetMove = getMove;
}

It's worth having subclasses when there are two abstract methods, but when there's just one it's less code (only one class instead of three) to use a delegate.

WhatHappensToMe is called twice and checks the rules twice. Instead, what happens to me is the complement of what happens to them; if you define the enum values as -1, 0, and 1 then you could simply negate the value.

GetReason checks the rules yet again. Instead, maybe looking-up the rule should return the result and the reason at the same time.

\$\endgroup\$
4
  • \$\begingroup\$ Your different Dictionary implementation would work, if Monkey and Banana weren't so weird. In standard RPSLS I would certainly go with that approach, but here Monkeys lose to each other, and Bananas win to each other instead of tie-ing. I see your point with the Player Abstraction, I'll think about that. And the reason WhatHappensToMe is called twice is because of the weird win-lose-tie cercumstances introduced by Monkey and Banana. GetReason looking up rules again I may indeed change if I implement a class structure for rules. Everything else I agree with :) \$\endgroup\$
    – BenVlodgi
    Commented Mar 17, 2014 at 17:08
  • 1
    \$\begingroup\$ In that case I'd suggest adding a bool? or an enum { WinAndLose, BothWin, BothLose } to the list of tuples I suggested: to make that explicit: because it's difficult to verify/inspect the data you have. Also, on loading the list, verify that the same (unordered) pair of gestures isn't defined more than once in the list. \$\endgroup\$
    – ChrisW
    Commented Mar 17, 2014 at 17:37
  • \$\begingroup\$ That is a valid point, and would replace what I have right now just fine. I may want to keep it this way in-case later I add other strange Gestures. You are correct, this current strucutre is difficult to verify \$\endgroup\$
    – BenVlodgi
    Commented Mar 17, 2014 at 19:23
  • 1
    \$\begingroup\$ "Hard to verify" is high on my list of priorities to fix: because a main reason for doing a code review is "verification". I want to look at code (and data) and verify (by inspection) that it's correct. Alternatively I'll look to see whether code is self-verifying (e.g. your code could have a run-time check/assertion that most pairs are defined twice: once as a winner and again as a loser). Or (last resort) I'll look for an exhaustive list of unit test cases. \$\endgroup\$
    – ChrisW
    Commented Mar 17, 2014 at 20: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.