Rock/Paper/Scissors using C#

Question

I created a Rock/Paper/Scissors game in C# that asks the player the number of rounds he wants to play, and then calculates the points and determines the winner. Is there a way I can fix my code? Is it appropriate to use switch and if/else together?

Console.Write("How many rounds do you want to play ? : ");
int numberOfRounds = Convert.ToInt32(Console.ReadLine());

Random rnd = new Random();
string[] hands = { "Rock", "Paper", "Sci" };
    //               0       1        2

int playerPoints = 0;
int computerPoints = 0;

for (int counter = 1; counter <= numberOfRounds; counter++)
{
    Console.Write("Player Hand : ");
    string playerHand = Console.ReadLine();
    int randomNumber = rnd.Next(0, 3);
    string computerHand = hands[randomNumber];
    Console.WriteLine("Computer Hand : " + computerHand);

    switch (playerHand)
    {
        case "Rock":
            if (computerHand == "Rock")
            {
                playerPoints++;
                computerPoints++;
                Console.WriteLine("Player And Computer Win a Point");
                Console.WriteLine();
            }
            if (computerHand == "Paper")
            {
                computerPoints++;
                Console.WriteLine("Computer wins a point");
                Console.WriteLine();
            }
            if (computerHand == "Sci")
            {
                playerPoints++;
                Console.WriteLine("Player wins a point");
                Console.WriteLine();
            }
            break;

        case "Paper":
            if (computerHand == "Paper")
            {
                playerPoints++;
                computerPoints++;
                Console.WriteLine("Player and Computer Win a Point");
                Console.WriteLine();
            }
            if (computerHand == "Sci")
            {
                computerPoints++;
                Console.WriteLine("Computer wins a point");
                Console.WriteLine();
            }
            if (computerHand == "Rock")
            {
                playerPoints++;
                Console.WriteLine("Player wins a point");
                Console.WriteLine();
            }
            break;

        case "Sci":
            if (computerHand == "Sci")
            {
                playerPoints++;
                computerPoints++;
                Console.WriteLine("Player and Computer Win a Point");
                Console.WriteLine();
            }
            if (computerHand == "Rock")
            {
                computerPoints++;
                Console.WriteLine("Computer wins a point");
                Console.WriteLine();
            }
            if (computerHand == "Paper")
            {
                playerPoints++;
                Console.WriteLine("Player wins a point");
                Console.WriteLine();
            }
            break;
    }
}

Console.WriteLine("*** RESULTS ***");
Console.WriteLine("Player Points : " + playerPoints);
Console.WriteLine("Computer Points : " + computerPoints);

if (playerPoints > computerPoints)
{
    Console.WriteLine("Player Wins");
}
else
{
    if (computerPoints > playerPoints)
    {
        Console.WriteLine("Computer Wins");
    }
    else
    {
        Console.WriteLine("Equal None Win");
    }
}

Console.ReadLine();

Answer 1

My take on this:

For console apps, I like to use a more OOP approach.

A Message class to hold a string array to handle a multiline message, and if the message requires a response a string of the characters allowed, will allow you to replace all those string literals with named variables:

class Message
{
    public String[] message { get; }
    public String choices { get; }
    public Message()
    {
        message = new String[0];
        choices = "";
    }
    public Message(String choices, params String[] message)
    {
        this.message = message;
        this.choices = choices.ToLower();
    }
}

To compliment this class, a static storage class(Messages) to put names to the various messages that will be required. I put a simple print method in there as well:

static class Messages
{
    public static Message startMessage = new Message("", "Welcome to RockScissorPaper");
    public static Message instructions = new Message("", "When you're ready to play, press the desired number.",
            "The computer will reveal its choice and who won.");
    public static Message rockWins = new Message("", "Rock breaks Scissors.", "Rock wins.");
    public static Message scissorsWins = new Message("", "Scissors cuts Paper.", "Scissors wins.");
    public static Message paperWins = new Message("", "Paper covers Rock.", "Paper wins.");
    public static Message playAgain = new Message("yn\n", "Would you like to play again?(y/n)");
    public static Message continuePlay = new Message("", "Press any key to continue.");
    public static Message choices = new Message("0123", "1 - Rock", "2 - Scissors", "3 - Paper", "0 - Quit", "\nWhich do you choose?");
    public static Message goodbye = new Message("", "Thank you for playing.  Goodbye.");
    public static Message tie = new Message("", "A tie.  You both chose the same.");
    public static Message wrongChoice = new Message("", "You made a wrong choice.  Try again.");

    public static void PrintMessage(TextWriter tOut, Message message)
    {
        tOut.WriteLine();
        foreach (var s in message.message)
        {
            tOut.WriteLine(s);
        }
    }
}

This approach makes it very easy, with minimal refactoring, to create language choices for your app.

For classes that require I/O, I like to keeps things flexible and use base classes instead of hard coding standard I/O. This makes it very easy to leverage networking at some point.

For the game class(RockScissorPaper), a method to get the user's input, clears a lot of code from the PlayGame method.

private static char GetChoice(TextWriter tOut, TextReader tIn, Message message)
{
    char choice = '\0';
    bool done = false;
    do
    {
        Messages.PrintMessage(tOut, message);
        string response = tIn.ReadLine().ToLower();
        choice = response.Length > 0 ? response[0] : '\n';
        if (message.choices.Contains(choice))
        {
            return choice;
        }
        Messages.PrintMessage(tOut, Messages.wrongChoice);
    } while (!done);
    return choice;
} 

I find an enum for the different states of the game, helps with the decision process to decide a winner:

private enum State
{
    None = 0,
    Tie = 1,
    Rock = 3,
    Scissors,
    Paper
}

Notice the Rock constant starts at 3. This allows simple math and one conditional to decide which player is the winner. Once the winner is decided the switch block to select which message to display is very simple:

private static Message GetWinner(State comp, State player)
{
    if (comp == State.None || player == State.None)
    {
        throw new ArgumentOutOfRangeException("Both 'comp' and 'player' must be 'Rock', 'Scissors' or 'Paper'.");
    }
    int cmptr = (int)comp;
    int plyr = (int)player;
    State winner = State.None;
    if (player == comp)
    {
        winner = State.Tie;
        plyrScore++;
        compScore++;
    }
    else if(plyr % 3 == (cmptr - 1) % 3)
    {
        winner = player;
        plyrScore += 2;
    }
    else
    {
        winner = comp;
        compScore += 2;
    }
    switch (winner)
    {
        case State.Tie:
            return Messages.tie;
        case State.Rock:
            return Messages.rockWins;
        case State.Scissors:
            return Messages.scissorsWins;
        case State.Paper:
            return Messages.paperWins;
        default:
            return new Message("", "");
    }
}

I've incorporated the score calculation here, as well. I noticed you scored a tie the same a win. Typically a tie is considered half a win. I would prefer to see a win at 2 points and a tie at 1.

With these helper methods done, the game play is almost trivial to code:

static class RockScissorsPaper
{
    private static RandomEngine rnd = new RandomEngine();
    private enum State
    {
        None = 0,
        Tie = 1,
        Rock = 3,
        Scissors,
        Paper
    }
    static int plyrScore = 0;
    static int compScore = 0;
    public static void PlayGame(TextWriter tOut, TextReader tIn, String name, int rounds)
    {
        plyrScore = 0;
        compScore = 0;
        Messages.PrintMessage(tOut, Messages.startMessage);
        for(int round = 0; round < rounds; ++round)
        {
            Messages.PrintMessage(tOut, Messages.instructions);
            int choice = GetChoice(tOut, tIn, Messages.choices) - '0';
            State compChoice = (State)Enum.GetValues(typeof(State)).GetValue((rnd.Next(0, 300) % 3) + 2);
            State playerChoice = (State)(choice + 2);
            tOut.WriteLine($"The player chose {playerChoice}.\nThe computer chose {compChoice}.");
            Messages.PrintMessage(tOut, GetWinner(compChoice, playerChoice));
            tOut.WriteLine($"The score is:\n{name} - {plyrScore}\nComputer - {compScore}");
            Messages.PrintMessage(tOut, Messages.continuePlay);
            tIn.Read();
        }

    }

    private static char GetChoice(TextWriter tOut, TextReader tIn, Message message)
    {
        char choice = '\0';
        bool done = false;
        do
        {
            Messages.PrintMessage(tOut, message);
            string response = tIn.ReadLine().ToLower();
            choice = response.Length > 0 ? response[0] : '\n';
            if (message.choices.Contains(choice))
            {
                return choice;
            }
            Messages.PrintMessage(tOut, Messages.wrongChoice);
        } while (!done);
        return choice;
    }
    private static Message GetWinner(State comp, State player)
    {
        if (comp == State.None || player == State.None)
        {
            throw new ArgumentOutOfRangeException("Both 'comp' and 'player' must be 'Rock', 'Scissors' or 'Paper'.");
        }
        int cmptr = (int)comp;
        int plyr = (int)player;
        State winner = State.None;
        if (player == comp)
        {
            winner = State.Tie;
            plyrScore++;
            compScore++;
        }
        else if(plyr % 3 == (cmptr - 1) % 3)
        {
            winner = player;
            plyrScore += 2;
        }
        else
        {
            winner = comp;
            compScore += 2;
        }
        switch (winner)
        {
            case State.Tie:
                return Messages.tie;
            case State.Rock:
                return Messages.rockWins;
            case State.Scissors:
                return Messages.scissorsWins;
            case State.Paper:
                return Messages.paperWins;
            default:
                return new Message("", "");
        }
    }
}

Answer 2

I've got a few suggestions to improve your application. I would suggest using a few functions to clean up the organization of your code. This will also allow you to move some off your logic away from your main application logic which will help to remove duplicate code.

First main change I would suggest is to move away from the array of strings to represent your choices. Right now you have to hard code string throughout your application. This will make it hard to make changes in the future and could cause errors if you misspell any of the values. Instead you could either use constant variable, which would allow you to change the value in a single place. See below:

    // Initial string representation
    public const string Rock = "Rock";
    public const string Paper = "Paper";
    public const string Scissor = "Scissor";

    // Later could be changed to this.
    public const int Rock = 1;
    public const int Paper = 2;
    public const int Scissor = 3;

Instead of using const variable we can use enums to store the player's actions as well as the applications state. This will also give us a couple benefits that will be shown below. The following code will be using these enums.

    public enum Action { Rock, Paper, Scissor };
    public enum Result { Tie, PlayerWon, ComputerWon }

When taking input from the user, you should be prepared for an incorrect input. The quick and easy way to handle parsing inputs is to use the 'TryParse' method found on the type you are trying to convert to. We can use the 'TryParse' method for both reading the amount of rounds the player desires and their hand choice.

    public static int GetDesiredRounds() {
        int result;
        do {
            var input = Console.ReadLine();
            if(Int32.TryParse(input, out result)) {
                return result;
            } else {
                Console.WriteLine("Invalid input. Please input a number.");
            }
        } while(true);
    }

Enums can also be parsed from a string using the 'TryParse' method. It also has an option to allow for case-insensitive parsing, which will allow the user to input either 'Rock' or 'rock' to get the same action.

    public static Action GetPlayerAction() {
        Action result;
        do {
            var input = Console.ReadLine();
            // The second param tells code to ignore case of string string
            if(Action.TryParse(input, true, out result)) {
                return result;  
            } else {
                Console.WriteLine("Invalid action {0}. Please input 'Rock', 'Paper' or 'Scissors'.", input);
            }
        } while(true);
    }

Enum's have an underlying numeric type. Their values default to 0, 1, 2, 3... from the first enum value to the last (unless they are manually assigned, see enum reference below). You can cast an Enum to and from their underlying type. Below we have to cast the enum value to its underlying type in order to use it with the random.Next method, then we simply cast it back to an Action.

    public static Action GetComputerAction() {
        // Gets a random number between 0 (Action.Rock) and 3 (Action.Scissor + 1)
        // This will result in the values 0 (Rock), 1 (Paper), or 2 (Scissor)
        // We use (Scissor + 1) since the 'Next' method does not include the max value
        return (Action)rand.Next((int)Action.Rock, (int)Action.Scissor + 1);
    }

With both the player and computer's choices, we then calculate the result. With the limited options, we could simply list out all the results using nested switch statements. One thing to observe is the throwing of an Exception. This exception should be never reached, since all the enum's values were used inside the switch. The compiler will complain if you remove the exception. (For a bit more information on this behavior see the reference link below).

    public static Result CalculateResult(Action player, Action computer) {
        switch(player) {
            case Action.Rock:
                switch (computer) ) {
                    case Action.Rock:    return Result.Tie;
                    case Action.Paper:   return Result.ComputerWon;
                    case Action.Scissor: return Result.PlayerWon;
                }
                break;
            case Action.Paper:
                switch (computer) ) {
                    case Action.Rock:    return Result.PlayerWon;
                    case Action.Paper:   return Result.Tie;
                    case Action.Scissor: return Result.ComputerWon;
                }
                break;
            case Action.Scissor:
                switch (computer) ) {
                    case Action.Rock:    return Result.ComputerWon;
                    case Action.Paper:   return Result.PlayerWon;
                    case Action.Scissor: return Result.Tie;
                }
                break;
        }

        // This should never be reached
        throw new Exception(string.Format("Unhandled action pair occured: {0}, {1}", player, computer) ));
    }

Finally we can use our created functions to build the main body of the application. Another thing to observe is since we pulled the logic of calculating results out from the printing and point code, there is less code to repeat.

    public static void Main() {
        // static variable used in GetComputerChoice
        rand = new Random();

        int playerPoints = 0;
        int computerPoints = 0;

        Console.WriteLine("How many rounds would you like to play?");
        int rounds = GetDesiredRounds();

        for(var round = 1; round <= rounds; round++) {
            Console.WriteLine("Round {0} Begins", round);
            Console.WriteLine("Which hand do you choose(Rock, Paper, Scissor)? ");

            var playerAction = GetPlayerAction();
            Console.WriteLine("You picked: {0}", playerAction.ToString());

            var computerAction = GetComputerAction();
            Console.WriteLine("Your opponent picked: {0}", computerAction.ToString());

            switch(CalculateResult(playerAction, computerAction)) {
                case Result.PlayerWon:
                    Console.WriteLine("You won the round! You gained a point.");
                    playerPoints++;
                    break;
                case Result.ComputerWon:
                    Console.WriteLine("Computer won the round! Computer gained a point.");
                    computerPoints++;
                    break;
                case Result.Tie:
                    Console.WriteLine("Round tied. You and the computer gained a point.");
                    playerPoints++;
                    computerPoints++;
                    break;
            }

            Console.WriteLine();
        }

        Console.WriteLine("Results - Player {0}, Computer {1}", playerPoints, computerPoints);
        if (playerPoints == computerPoints) {
            Console.WriteLine("Tie Game");  
        } else {
            bool isPlayerWinner = playerPoints > computerPoints;
            Console.WriteLine("{0} won the game!", isPlayerWinner ? "Player" : "Computer"); 
        }
    }

References

Ins and Outs of enums in C#

Enums in switch statements

Demo

The full example code can be found and ran here

Answer 3

Right at the start we can turn your hands array in to a dictionary so we can compare Int insted of String, which is way faster.

In such a small application it is not mandatory such optimizations, but it is good to consider in the long run.

Dictionary<string, int> handsValues = new Dictionary<string, int>()
            {
                {"Rock", 0},
                {"Paper", 1},
                {"Sci", 2}
            };

Because we are going to compare only Int values we get rid of:

int randomNumber = rnd.Next(0, 3);
string computerHand = hands[randomNumber];
Console.WriteLine("Computer Hand : " + computerHand); 

We turn it into:

int computerHand = rnd.Next(0, 3);
Console.WriteLine("Computer Hand : " + handsValues.Keys.ElementAt(computerHand));

Calling .Keys on a dictionary returns to us a collection of Index,Value pairs Then calling .ElementAt(int index) we can get a value by its index

---

The only clear problem with your code is the computerHand and playerHand logic, you can first check for equality of the hands before going to check each variation.

if (handsValues[playerHand] == computerHand)
{
    playerPoints++;
    computerPoints++;
    Console.WriteLine("Player And Computer Win a Point" + Environment.NewLine);
}
else
{
    switch (handsValues[playerHand])
    {
        case 0:
            if (computerHand == 1)
            {
                computerPoints++;
                Console.WriteLine("Computer wins a point" + Environment.NewLine);
            }
            else
            {
                playerPoints++;
                Console.WriteLine("Player wins a point" + Environment.NewLine);
            }
            break;

        case 1:
            if (computerHand == 2)
            {
                computerPoints++;
                Console.WriteLine("Computer wins a point" + Environment.NewLine);
            }
            else
            {
                playerPoints++;
                Console.WriteLine("Player wins a point" + Environment.NewLine);
            }
            break;

        case 2:
            if (computerHand == 0)
            {
                computerPoints++;
                Console.WriteLine("Computer wins a point" + Environment.NewLine);
            }
            else
            {
                playerPoints++;
                Console.WriteLine("Player wins a point" + Environment.NewLine);
            }
            break;
    }
}

---

A small detail, you can skip the Console.WriteLine();, by putting "\r\n" at the end of you string(not recommended due to OS compatibility problems) or by concatenating Environment.NewLine to your string.

Your naming of variable is on point, excluding the unclear hands

The full code you can check HERE, USE Ctrl + Click