patterncsharpMinor
Rock-Paper-Scissors-Lizard-Spock Challenge, take 2
Viewed 0 times
scissorspaperchallengespocktakerocklizard
Problem
This post is following-up on Rock-Paper-Scissors-Lizard-Spock Challenge
I had a gut feeling that I was somehow abusing
As @dreza was posting his answer I was in the process of proceeding with exactly that refactoring: stuffing the
Building on @ChrisWue's answer, I came up with the following code:
Selections
Turned into enums:
GameRule
Designed to be represented as a string that looks exactly how Sheldon enumerates them:
GameResult
Rather than going
```
public class GameResult
{
private readonly GameRule _outcome;
private readonly Selection _player;
private GameResult(GameRule outcome, Selection player)
{
_outcome = outcome;
I had a gut feeling that I was somehow abusing
IComparable, @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;
Solution
-
I chose an Evaluate factory method with a private constructor
I think that
-
I think this would be cleaner as several separate
-
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 agree. I think a better solution would be to create a structure that maps numbers to
You could use this structure both in
-
These are incorrect grammatically, they should be
I chose an Evaluate factory method with a private constructor
I think that
GameResult.Evaluate() doesn't logically make much sense, this logic belongs to a method in Game, which means the constructor of GameResult would be public. Also, why does GameResult contain Selection player? Something like bool playerWon would be enough.-
Console.WriteLine("Rock-Paper-Scissors-Lizard-Spock 1.1\n{0}\n", new string('=', 40));I think this would be cleaner as several separate
WriteLines:Console.WriteLine("Rock-Paper-Scissors-Lizard-Spock 1.1");
Console.WriteLine(new string('=', 40));
Console.WriteLine();-
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 agree. I think a better solution would be to create a structure that maps numbers to
Selections. Dictionary is probably not a good idea, because you want to output the selections in order and Dictionary is unordered. Instead, you can use for example List:var selections = Enum.GetValues(typeof(Selection));
_selectionsMap = Enumerable.Range(1, selections.Length)
.Zip(selections,
(index, selection) => new KeyValuePair(index, selection));You could use this structure both in
LayoutGameScreen() and GetUserSelection().-
new GameRule(Selection.Scissors, "cuts", Selection.Paper)
new GameRule(Selection.Scissors, "decapitates", Selection.Lizard)These are incorrect grammatically, they should be
"cut" and "decapitate".Code Snippets
Console.WriteLine("Rock-Paper-Scissors-Lizard-Spock 1.1\n{0}\n", new string('=', 40));Console.WriteLine("Rock-Paper-Scissors-Lizard-Spock 1.1");
Console.WriteLine(new string('=', 40));
Console.WriteLine();var selections = Enum.GetValues(typeof(Selection));
_selectionsMap = Enumerable.Range(1, selections.Length)
.Zip(selections,
(index, selection) => new KeyValuePair<int, Selection>(index, selection));new GameRule(Selection.Scissors, "cuts", Selection.Paper)
new GameRule(Selection.Scissors, "decapitates", Selection.Lizard)Context
StackExchange Code Review Q#36426, answer score: 8
Revisions (0)
No revisions yet.