This post is following-up on Rock-Paper-Scissors-Lizard-Spock Challenge
I had a gut feeling that I was somehow abusing IComparable<T>, @svick's answer confirmed that.
As @dreza was posting his answer I was in the process of proceeding with exactly that refactoring: stuffing the SelectionBase class with most of the functionality, effectlvely turning the derived classes into more or less useless, combersome objects that would be best implemented as an enum.
Building on @ChrisWue's answer, I came up with the following code:
Selections
Turned into enums:
public enum Selection
{
Rock,
Paper,
Scissors,
Lizard,
Spock
}
GameRule
Designed to be represented as a string that looks exactly how Sheldon enumerates them:
public class GameRule
{
private readonly Selection _winner;
private readonly Selection _loser;
private readonly string _verb;
public GameRule(Selection winner, string verb, Selection loser)
{
_winner = winner;
_loser = loser;
_verb = verb;
}
public Selection Winner { get { return _winner; } }
public Selection Loser { get { return _loser; } }
public override string ToString()
{
return string.Format("{0} {1} {2}", _winner, _verb, _loser);
}
}
GameResult
Rather than going static all the way, I chose an Evaluate factory method with a private constructor, and instead of having a bool that tracks whether the player wins, I've included the player's selection as a parameter of that constructor. The InvalidOperationException thrown in the ToString method is never hit, but it satisfies the compiler's need for all paths to return a value while eliminating a "default" else branch that would make a result be implicitly defined:
public class GameResult
{
private readonly GameRule _outcome;
private readonly Selection _player;
private GameResult(GameRule outcome, Selection player)
{
_outcome = outcome;
_player = player;
}
public override string ToString()
{
if (_outcome == null) return "Meh. Tie!";
if (_player == _outcome.Winner) return string.Format("{0}. You win!", _outcome);
if (_player == _outcome.Loser) return string.Format("{0}. You lose!", _outcome);
throw new InvalidOperationException();
}
public static GameResult Evaluate(IEnumerable<GameRule> rules, Selection player, Selection sheldon)
{
var rule = rules.FirstOrDefault(r => r.Winner == player && r.Loser == sheldon || r.Winner == sheldon && r.Loser == player);
return new GameResult(rule, player);
}
}
This invited refactoring my Game class to look like this:
public class Game
{
private readonly IUserInputProvider _consoleInput;
private readonly IResultWriter _resultWriter;
private readonly IEnumerable<GameRule> _rules;
public Game(IUserInputProvider console, IResultWriter resultWriter, IEnumerable<GameRule> rules)
{
_consoleInput = console;
_resultWriter = resultWriter;
_rules = rules;
}
public void Run()
{
while (true)
{
LayoutGameScreen();
var player = GetUserSelection();
if (player == null) return;
var sheldon = Selection.Spock;
var result = GameResult.Evaluate(_rules, player.Value, sheldon);
_resultWriter.OutputResult(result);
Pause();
}
}
private void Pause()
{
Console.WriteLine("\nPress <ENTER> to continue.");
Console.ReadLine();
}
private void LayoutGameScreen()
{
Console.Clear();
Console.WriteLine("Rock-Paper-Scissors-Lizard-Spock 1.1\n{0}\n", new string('=', 40));
foreach (var item in Enum.GetValues(typeof(Selection)))
Console.WriteLine("\t[{0}] {1}", (int)item + 1, item);
Console.WriteLine();
}
private Selection? GetUserSelection()
{
var playable = Enum.GetValues(typeof(Selection));
var values = playable.Cast<int>()
.Select(item => (item + 1).ToString())
.ToList();
values.Add(string.Empty); // allows a non-selection
var input = _consoleInput.GetValidUserInput("Your selection?", values);
if (input == string.Empty)
{
var confirm = _consoleInput.GetValidUserInput("Giving up already? (Y/N)", new[] { "Y", "N" });
if (confirm.ToUpper() == "Y") return null;
}
int selection;
var isValidSelection = int.TryParse(input, out selection) && playable.Cast<int>().Contains(selection - 1);
if (!isValidSelection) return GetUserSelection();
return playable.Cast<Selection>().Single(item => (int)item == selection - 1);
}
}
One thing I'm not sure I like, is the +1/-1 and the casting going on here, in order to display 1-based menu items; I refuse to explicitly assign an int value to each possible Selection.
The refactoring has greatly simplified the ConsoleResultWriter class:
public interface IResultWriter
{
void OutputResult(GameResult outcome);
}
public class ConsoleResultWriter : IResultWriter
{
public void OutputResult(GameResult outcome)
{
Console.WriteLine("\n\tSheldon says: \"{0}\"\n", outcome);
}
}
...and left the Main method looking like this:
static void Main(string[] args)
{
var consoleReader = new ConsoleUserInputProvider();
var consoleWriter = new ConsoleResultWriter();
var rules = new[] {
new GameRule(Selection.Scissors, "cuts", Selection.Paper),
new GameRule(Selection.Paper, "covers", Selection.Rock),
new GameRule(Selection.Rock, "crushes", Selection.Lizard),
new GameRule(Selection.Lizard, "poisons", Selection.Spock),
new GameRule(Selection.Spock, "smashes", Selection.Scissors),
new GameRule(Selection.Scissors, "decapitates", Selection.Lizard),
new GameRule(Selection.Lizard, "eats", Selection.Paper),
new GameRule(Selection.Paper, "disproves", Selection.Spock),
new GameRule(Selection.Spock, "vaporizes", Selection.Rock),
new GameRule(Selection.Rock, "crushes", Selection.Scissors)
};
var game = new Game(consoleReader, consoleWriter, rules);
game.Run();
}
AI aside, can it get any better? Is there anything weird still going on? How's the naming? Any KISS-shredding interfaces?
