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

RPSLS Game in C#

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
gamerpslsstackoverflow

Problem

I went with what I know and can use well, not what I know and can't figure the syntax out to make it look good.

So please enjoy the code and prepare to school me (probably in the basics) in C#

```
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 listOfGestures = new List();
listOfGestures.Add("rock");
listOfGestures.Add ("paper");
listOfGestures.Add ("scissors");
listOfGestures.Add ("lizard");
listOfGestures.Add ("spock");

int win = 0;
int lose = 0;
int tie = 0;

var newGame = true;
while (newGame)
{
var playerGesture = "";
var validInput = false;
while (!validInput)
{
var playerChoice = -1;
Console.WriteLine("Please choose your Gesture ");
gameSetup(listOfGestures);
try
{
playerChoice = Convert.ToInt32 (Console.ReadLine());
}
catch (FormatException e)
{
validInput = false;
}

if (playerChoice > 0 && playerChoice <= listOfGestures.Count)
{
playerGesture = listOfGestures[playerChoice - 1];
validInput = true;
}
else
{
validInput = false;
}
}

var computerPlay = ComputerGesture(listOfGestures);

Console.WriteLine ("Computer: " + computerPlay);
Console.WriteLine ("your Gesture: " + playerGesture);

if (playerGesture == computerPlay)
{
tie++;
Console.WriteLine ("you have tied with the computer");
Console.W

Solution

-
Some minor syntax detail: C# has collection initializers so one can write:

List listOfGestures = new List {
    "rock", "paper", "scissors", "lizard", "spock" 
}


-
You use magic strings for the gestures all over the place. This can pose a problem if you want to change them. You could do a find and replace but this could inadvertently change a part of a method or variable name which happens to include "rock" if you are not careful.

In other solutions to the problem posted before enums were used instead. If you insist on using strings then make them named constants and use those instead

const string Rock = "rock";
const string Paper = "paper";
... etc.


-
The main problem is that the entire implementation is basically just one big blob of code in the main method. Logic and input/output are totally intertwined. Assume you have to write an automated test case for this - it will drive you insane.

If I were to encounter code like this in production I would very likely kill it and rewrite it - the problem is fairly simple and given it's untestable there aren't any unit tests anyway. If the code were too much to rewrite then I'd start refactoring it. Possible steps:

-
Create a Game class which effectively encapsulates the main while loop up until the line Console.WriteLine ("Your Score is (W:L:T:) : {0}:{1}:{2}", win, lose, tie);

  • Dump everything into one method into that class for starters.



  • The Game class should also keep track of the score.



  • Override ToString to print out the current scores.



  • Move the "ask user if he wants to play another round" into a method on the Game class



After that the Main should look somewhat like this:

var game = new Game();
var continuePlaying = true;
while (continuePlaying)
{
    game.Play();
    Console.WriteLine(game.ToString());
    continuePlaying = game.ShouldContinue();
}


-
Next step would be to start refactoring the Game class and the methods within.

-
Move the decision logic into it's own internal private method to reduce the bulk of the Play() method.

-
Define an interface for getting user input and another one for writing output. Pass these interfaces into the Game constructor. Provide a console implementation for both interfaces. Change all the input and output to use the interfaces rather than the console.

-
Now Game is decoupled from the specific input and output methods and you can start writing unit tests for the Play() and the ShouldContinue() methods. Once you have done that you can continue refactoring those methods. And because you have just created unit tests you should be able to detect if you create any regressions (introducing bugs) while doing so.

-
Wash, Rinse, Repeat

Code Snippets

List<string> listOfGestures = new List<string> {
    "rock", "paper", "scissors", "lizard", "spock" 
}
const string Rock = "rock";
const string Paper = "paper";
... etc.
var game = new Game();
var continuePlaying = true;
while (continuePlaying)
{
    game.Play();
    Console.WriteLine(game.ToString());
    continuePlaying = game.ShouldContinue();
}

Context

StackExchange Code Review Q#36482, answer score: 18

Revisions (0)

No revisions yet.