patterncsharpModerate
RPSLS is less messy now, but is it clean?
Viewed 0 times
butmessyrpslsnowlessclean
Problem
This is a follow up to the following questions:
RPSLS Game in C#
Ensuring user input is an integer in a range
I haven't made my way to
I did not change to using an Enumeration for my
I also used
I am not afraid to say that I am still learning, so here is the code:
```
///
/// Thanks to
/// https://codereview.stackexchange.com/users/38054/benvlodgi
/// https://codereview.stackexchange.com/users/4318/eric-lippert
/// https://codereview.stackexchange.com/users/23788/mats-mug
/// https://codereview.stackexchange.com/users/30346/chriswue
///
namespace RPSLSGame
{
class MainClass
{
public static void Main (string[] args)
{
/* Here are your rules:
"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.
And as it always has, rock crushes scissors."
-- Dr. Sheldon Cooper */
List Gestures = new List{"rock","paper","scissors", "lizard","spock"};
int win = 0;
int lose = 0;
int tie = 0;
var newGame = true;
while (newGame)
{
var playerGesture = "";
Console.WriteLine("Please choose your Gesture ");
RPSLS Game in C#
Ensuring user input is an integer in a range
I haven't made my way to
DecideWinner() yet, but that is next on my To-Do List. I would like the majority of the focus to go into what I have done with the rest of the code, but would still enjoy some input on what to do with my nasty if then else statements. I did not change to using an Enumeration for my
Gestures. I read the Microsoft page for Enumeration and it didn't seem like it would be a good fit, maybe I didn't understand fully though either.I also used
while loops instead of do while loops because I like them better. not really a good reason I guess other than that I prefer while loops.I am not afraid to say that I am still learning, so here is the code:
```
///
/// Thanks to
/// https://codereview.stackexchange.com/users/38054/benvlodgi
/// https://codereview.stackexchange.com/users/4318/eric-lippert
/// https://codereview.stackexchange.com/users/23788/mats-mug
/// https://codereview.stackexchange.com/users/30346/chriswue
///
namespace RPSLSGame
{
class MainClass
{
public static void Main (string[] args)
{
/* Here are your rules:
"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.
And as it always has, rock crushes scissors."
-- Dr. Sheldon Cooper */
List Gestures = new List{"rock","paper","scissors", "lizard","spock"};
int win = 0;
int lose = 0;
int tie = 0;
var newGame = true;
while (newGame)
{
var playerGesture = "";
Console.WriteLine("Please choose your Gesture ");
Solution
I like the
However I don't like the multi-screen
What if you had a method to handle each of the player's possible moves? Say you have a method to increment losses and another to increment wins - you'd have to promote
Then you could have a method to handle each player moves:
And then your main loop could "branch" on a method depending on the player's move - you can set up a
This way you can invoke the appropriate
...this invocation replaces the entire
while loop. It states the exit condition from the start, so you know right away what you're getting yourself into.However I don't like the multi-screen
if block.What if you had a method to handle each of the player's possible moves? Say you have a method to increment losses and another to increment wins - you'd have to promote
win, lose and tie to instance variables for this to work:private void PlayerWins(string playerGesture, string verb, string computerGesture)
{
win++;
Console.WriteLine ("You Win, {0} {1} {2}", playerGesture, verb, computerGesture);
}
private void ComputerWins(string computerGesture, string verb, string playerGesture)
{
lose++;
Console.WriteLine ("You Lose, {0} {1} {2}", computerGesture, verb, playerGesture);
}
private void NobodyWins()
{
tie++;
Console.WriteLine ("Tie!");
}Then you could have a method to handle each player moves:
private void PlayRock(string computerGesture)
{
var playerGesture = "rock";
if (computerGesture == "lizard" || computerGesture == "scissors")
{
PlayerWins(playerGesture, "crushes", computerGesture);
}
else if (computerGesture == "paper")
{
ComputerWins(computerGesture, "covers", playerGesture);
}
else if (computerGesture == "spock")
{
ComputerWins(computerGesture, "vaporizes", playerGesture);
}
}And then your main loop could "branch" on a method depending on the player's move - you can set up a
Dictionary> to do that (Action is a delegate that returns void and takes a string parameter.. which matches the signatures of your PlayXXXXX methods):var plays = new Dictionary> {
{ "rock", PlayRock },
{ "paper", PlayPaper },
{ "scissors", PlayScissors },
{ "lizard", PlayLizard },
{ "spock", PlaySpock }
};This way you can invoke the appropriate
Action delegate simply by getting the dictionary entry for the playerGesture:plays[playerGesture](computerGesture);...this invocation replaces the entire
if block in the main loop, and as a bonus you now have a separate method for each playable move.Code Snippets
private void PlayerWins(string playerGesture, string verb, string computerGesture)
{
win++;
Console.WriteLine ("You Win, {0} {1} {2}", playerGesture, verb, computerGesture);
}
private void ComputerWins(string computerGesture, string verb, string playerGesture)
{
lose++;
Console.WriteLine ("You Lose, {0} {1} {2}", computerGesture, verb, playerGesture);
}
private void NobodyWins()
{
tie++;
Console.WriteLine ("Tie!");
}private void PlayRock(string computerGesture)
{
var playerGesture = "rock";
if (computerGesture == "lizard" || computerGesture == "scissors")
{
PlayerWins(playerGesture, "crushes", computerGesture);
}
else if (computerGesture == "paper")
{
ComputerWins(computerGesture, "covers", playerGesture);
}
else if (computerGesture == "spock")
{
ComputerWins(computerGesture, "vaporizes", playerGesture);
}
}var plays = new Dictionary<string,Action<string>> {
{ "rock", PlayRock },
{ "paper", PlayPaper },
{ "scissors", PlayScissors },
{ "lizard", PlayLizard },
{ "spock", PlaySpock }
};plays[playerGesture](computerGesture);Context
StackExchange Code Review Q#44034, answer score: 10
Revisions (0)
No revisions yet.