Problem
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;
}
}
Solution
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.