HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

RPSLS is less messy now, but is it clean?

Submitted by: @import:stackexchange-codereview··
0
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 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 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.