RockPaperScissor Game

Question

I have been studying C#, and I am trying to model the "Rock / Paper / Scissors" game.

Is my program reasonable or is there anything I need to improve?

1. user input a round.
2. user input RockPaper or Scissor.
3. After the game is finished , user can choose to play again or quit.

namespace Project_RockPaperSci_Revised
{
    public class RockPaperScissor
    {
        public Random rnd = new Random();
        public string userInput;
        public int userScore = 0;
        public int comScore = 0;
        public bool isPlaying = true;
        public int around;

        public void PlayGame()
        {
            Console.WriteLine("Welcome Let's play Rock Paper Scissors");

            SetRound(ref around);

            while (isPlaying) {

                userScore = 0;
                comScore = 0;

                while (userScore < around && comScore < around) {

                    PlayerInput(ref userInput);
                    Console.WriteLine("\nPlayer chose {0}", userInput);
                    Match(userInput, ref userScore, ref comScore);
                    Console.WriteLine($"User's Score<{userScore}>\tCom's Score<{comScore}>\n");

                }

                string again = string.Empty;
                Console.WriteLine("Do you want to play again? y/n ");
                again = Console.ReadLine();
                again = again.ToUpper();
                if (again == "Y")
                {
                    Console.Clear();

                } else if (again == "N")
                {
                    Console.WriteLine("Good Bye");
                    isPlaying = false;
                }
            }
        }
        public string PlayerInput(ref string userInput)
        {
            Console.Write("Choose btw RockPaperScissor \n");
            userInput = Console.ReadLine();
            userInput = userInput.ToUpper();
            return userInput;
        }
        public int ShowResult(int flagNum)
        {
            int flagScore =0 ;
            switch (flagNum)
            {
                case 1:
                    Console.WriteLine("User Win !!");
                    break;
                case 2:
                    Console.WriteLine("Computer Win !!");
                    break;
                case 3:
                    Console.WriteLine("Draw !!");
                    break;
                default:
                    break;
            }
            return flagScore;
        }
        public void Match(string userInput,ref int userScore, ref int comScore)
        {
            int comChoice;
            comChoice = rnd.Next(1, 4);
            // flagNum 1 = userWin    2 = ComWin      3 = Draw     
            int flagNum = 0;

            switch (comChoice)
            {  
                case 1:
                    Console.WriteLine("Computer chose ROCK");
                    if (userInput == "ROCK")
                    {
                        flagNum = 3;
                    }else if (userInput == "PAPER")
                    {
                        flagNum = 1;
                        comScore++;
                    } else if (userInput == "SCISSOR")
                    {
                        flagNum = 2;
                        userScore++;
                    }
                    break;
                case 2:
                    Console.WriteLine("Computer chose PAPER");
                    if (userInput == "ROCK")
                    {
                        flagNum = 2;
                        comScore++;
                    }
                    else if (userInput == "PAPER")
                    {
                        flagNum = 3;
                    }
                    else if (userInput == "SCISSOR")
                    {
                        flagNum = 1;
                        userScore++;
                    }
                    break;
                case 3:
                    Console.WriteLine("Com Chose SCISSOR");
                    if (userInput == "ROCK")
                    {
                        flagNum = 1;
                        userScore++;
                    }
                    else if (userInput == "PAPER")
                    {
                        flagNum = 2;
                        comScore++;
                    }
                    else if (userInput == "SCISSOR")
                    {
                        flagNum = 3;
                    }
                    break;
                default:
                    Console.WriteLine("Error");
                    break;
            }
            ShowResult(flagNum);
        }
        public void SetRound(ref int around)
        {
            Console.Write("How many round would you like to play?\t");
            around = Convert.ToInt32(Console.ReadLine());
        }
    }
    class Program
    {
        static void Main(string[] args)
        {
            RockPaperScissor rps1 = new RockPaperScissor();
            rps1.PlayGame();
        }
    }
}

Answer 1

You're using int as flags, which is what enum stands for! Instead, why don't we define two:

enum Choice
{
    Rock,
    Paper,
    Scissors
}

enum Player
{
    User,
    Computer
}

Now that you have all the information, let's compute who win a round:

static Player? ComputeWinner(Choice userChoice, Choice computerChoice)
{
    if (userChoice == computerChoice)
        return null;

    switch (computerChoice)
    {
        case Choice.Rock:
            switch (userChoice)
            {
                case Choice.Paper:
                    return Player.User;
                case Choice.Scissors:
                    return Player.Computer;
            }
            break;
        case Choice.Paper:
            switch (userChoice)
            {
                case Choice.Rock:
                    return Player.User;
                case Choice.Scissors:
                    return Player.Computer;
            }
            break;
        case Choice.Scissors:
            switch (userChoice)
            {
                case Choice.Rock:
                    return Player.User;
                case Choice.Paper:
                    return Player.Computer;
            }
            break;
    }

    return null;
}

What defines a game? A player score, a computer score and a winner. You put it as variables, I would suggest to make a class, named Game. Here we go:

class Game
{
    public int PlayerScore { get; set; }
    public int ComputerScore { get; set; }
    public override string ToString()
    {
        return $"User's Score<{PlayerScore}>\tCom's Score<{ComputerScore}>\n";
    }

    public Player Winner => PlayerScore > ComputerScore ? Player.User : Player.Computer;
}

Now the match display:

static void Match(string userInput, Game game)
{
    // Adds some security around user input
    if (!Enum.TryParse(userInput, out Choice userChoice))
    {
        Console.WriteLine($"{userInput} is not valid");
        return;
    }

    // Let the computer pick a value
    var choices = Enum.GetValues(typeof(Choice));
    var random = new Random();
    var comChoice = (Choice)choices.GetValue(random.Next(choices.Length));

    Console.WriteLine($"Computer chose {comChoice}");

    switch (ComputeWinner(userChoice, comChoice))
    {
        case Player.User:
            Console.WriteLine("User won");
            game.PlayerScore++;
            break;
        case Player.Computer:
            Console.WriteLine("Computer won");
            game.ComputerScore++;
            break;
        default:
            Console.WriteLine("Tie");
            break;
    }
}

And here's the wrapping, in order to play as many games as you wish:

public static void PlayGame()
{
    Console.WriteLine("Welcome Let's play Rock Paper Scissors");

    int around = SetRound();

    // initiate a new Game
    var game = new Game();

    // Allowed since we put a "break" if the answer is N
    while (true)
    {
        while (game.PlayerScore < around && game.ComputerScore < around)
        {

            var userInput = PlayerInput();
            Console.WriteLine("\nPlayer chose {0}", userInput);

            Match(userInput, game);

            Console.WriteLine();
        }

        Console.WriteLine($"{game.Winner} won !");

        Console.WriteLine("Do you want to play again? y/n ");
        var again = Console.ReadLine()?.ToUpper();
        if (again == "N")
        {
            Console.WriteLine("Good Bye");
            break;
        }
        // Clear the screen if the user want to play more
        Console.Clear();

    }
}

Which gives:

public class RockPaperScissor
{
    class Game
    {
        public int PlayerScore { get; set; } = 0 ;
        public int ComputerScore { get; set; } = 0;
        public override string ToString()
        {
            return $"User's Score<{PlayerScore}>\tCom's Score<{ComputerScore}>\n";
        }

        public Player Winner => PlayerScore > ComputerScore ? Player.User : Player.Computer;
    }

    enum Choice
    {
        Rock,
        Paper,
        Scissors
    }

    enum Player
    {
        User,
        Computer
    }

    public static void PlayGame()
    {
        Console.WriteLine("Welcome Let's play Rock Paper Scissors");

        int around = SetRound();
        var game = new Game();

        while (true)
        {
            while (game.PlayerScore < around && game.ComputerScore < around)
            {

                var userInput = PlayerInput();
                Console.WriteLine("\nPlayer chose {0}", userInput);

                Match(userInput, game);

                Console.WriteLine();
            }

            Console.WriteLine($"{game.Winner} won !");

            Console.WriteLine("Do you want to play again? y/n ");
            var again = Console.ReadLine()?.ToUpper();
            if (again == "N")
            {
                Console.WriteLine("Good Bye");
                break;
            }
            Console.Clear();
        }
    }
    public static string PlayerInput()
    {
        Console.Write("Choose btw RockPaperScissor \n");
        var input = Console.ReadLine();

        if (input == null)
            return null;

        return Thread.CurrentThread.CurrentCulture.TextInfo.ToTitleCase(input.ToLower());
    }

    static Player? ComputeWinner(Choice userChoice, Choice computerChoice)
    {
        if (userChoice == computerChoice)
            return null;

        switch (computerChoice)
        {
            case Choice.Rock:
                switch (userChoice)
                {
                    case Choice.Paper:
                        return Player.User;
                    case Choice.Scissors:
                        return Player.Computer;
                }
                break;
            case Choice.Paper:
                switch (userChoice)
                {
                    case Choice.Rock:
                        return Player.User;
                    case Choice.Scissors:
                        return Player.Computer;
                }
                break;
            case Choice.Scissors:
                switch (userChoice)
                {
                    case Choice.Rock:
                        return Player.User;
                    case Choice.Paper:
                        return Player.Computer;
                }
                break;
        }

        return null;
    }


    static void Match(string userInput, Game game)
    {
        if (!Enum.TryParse(userInput, out Choice myStatus))
        {
            Console.WriteLine($"{userInput} is not valid");
            return;
        }

        var userChoice = myStatus;

        var values = Enum.GetValues(typeof(Choice));
        var random = new Random();
        var comChoice = (Choice)values.GetValue(random.Next(values.Length));

        Console.WriteLine($"Computer chose {comChoice}");

        switch (ComputeWinner(userChoice, comChoice))
        {
            case Player.User:
                Console.WriteLine("User won");
                game.PlayerScore++;
                break;
            case Player.Computer:
                Console.WriteLine("Computer won");
                game.ComputerScore++;
                break;
            default:
                Console.WriteLine("Tie");
                break;
        }
    }
    public static int SetRound()
    {
        Console.Write("How many round would you like to play?\t");
        return Convert.ToInt32(Console.ReadLine());
    }
}

Answer 2

Passing class fields/properties as ref

You're using ref to pass a variable and update it.

public void SetRound(ref int around)
{
    Console.Write("How many round would you like to play?\t");
    around = Convert.ToInt32(Console.ReadLine());
}

This is unnecessary, as in all of these occurances, the variables in question are class fields or properties. Instead you can do as follows:

public void SetRound()
{
    Console.Write("How many round would you like to play?\t");
    around = Convert.ToInt32(Console.ReadLine());
}

The same goes for Match. If you want to be more explicit about updating the class variable, use this.around instead.

The logic inside match:

The switch and if statements are pretty ugly and use a lot of common logic. First let's start with changing the strings to an enum:

public enum Choices {
    Rock,
    Paper,
    Scissors
}

We can then define which choice beats which:

Dictionary<Choices, Choices> Beats = new Dictionary<Choices, Choices> {
    { Choices.Rock, Choices.Scissors },
    { Choices.Scissors, Choices.Paper },
    { Choices.Paper, Choices.Rock }
};

We can then define an enum based on who has won that round:

public enum WhoWins {
    User,
    Com,
    Draw
}

Then the method becomes:

public WhoWins Match(Choices userChoice) {
    Choices comChoice = (Choices)rnd.Next(0, 3);

    if(comChoice == userChoice) {
        return WhoWins.Draw;
    } else if (Beats[userChoice] == comChoice) {
        return WhoWins.User;
    } else {
        return WhoWins.Com;
    }
}

First you can check for draws simply by comparing the two choices. Then you can use the defined dictionary to see if the choices match. By exhaustion, if they don't match, that means that the user lost.

This way, the game is quite easily extendible to more choices, like Lizard or Spock, simply by increasing the values in the dictionary.

Answer 3

I gave this a very quick review while still on my first cup of coffee. The answer is not very detailed, but then there were a lot of things I caught even with a quick scan.

The public fields at the top of the RockPaperScissor class should either be private fields or public properties. The one exception to public fields are constants. Note a static readonly value cannot be a constant, so if you have any of those they should be properties (be it public or private).

If you use public properties, the naming should be Pascal-cased, so isPlaying should be IsPlaying (if you think it should be exposed publicly). Private fields or properties should begin with lowercase; public fields or properties should begin with uppercase.

isPlaying is initialized to true. It should be false and you should set it to true at the top of PlayGame. You may want to put a check at the top to make sure a game is not already in progress.

I'd like to see a enum for { Rock, Paper, Scissors } instead of literals "ROCK", "PAPER", and "SCISSORS". This requires better validation for the user input to map that input to the correct enum. I would also recommend using a switch instead of cascading if-else-if's.

ShowResult(int flagNum) could be improved. I don't like flagNum as an integer and would prefer to see an enum of GameResult = { InProgress, ComputerWins, PlayerWins, Draw }. There should be a class level public readonly property to hold the GameResult. And ShowResult should not take any parameters but instead read that class level public readonly property.

Answer 4

1) The PlayerInput method doesn't need to have a parameter as userInput is declared in a global scope.

2) Instead of having flagNum as an in, you should create an enum called something like RoundWinner and give it all possible ends for a round (Computer, Player, None).

3) In the PlayerInput() method you can shorten

userInput = Console.ReadLine();
userInput = userInput.ToUpper();
return userInput;

to return Console.ReadLine().ToUpper();.

4) isPlaying is useless, instead you can change your while to while(true) and break where you were doing isPlaying = false;.

5) Same problem than in 3) : in the PlayGame() method you can shorten

string again = string.Empty;
again = Console.ReadLine();
again = again.ToUpper();

to again = Console.ReadLine().ToUpper();.

6) The ShowResult() method doesn't need to return an int.

7) You should also have an enum for rock scissors and paper which would be named like Weapons instead of comparing strings directly.

8) Same problem than 1) : your Match() method doesn't need userScore and comScore arguments as they are declared in a global scope.

9) Same problem than 1) and 8) : your SetRound() method doesn't need around as argument as it is declared in a global scope.

Answer 5

This logic handling is much simpler and reflects the cyclical nature of matchups

    enum Choices
    {
        Rock = 0,
        Paper = 1,
        Scissors = 2
    }

    enum Winner
    {
        Draw,
        User,
        Com
    }

    static readonly int ChoicesCount = Enum.GetNames(typeof(Choices)).Length;
    static Winner Matrix(Choices player, Choices opponent)
    {
        return (Winner) (((int) player - (int) opponent + ChoicesCount) % ChoicesCount);
    }

You could even make ChoicesCount a const int but I find it more maintainable if it just calculates from Choices.

Rock-Paper-Scissors-Lizard-Spock, as brought up in a comment would be implemented by adding 2 more options to Choices, and then modifying Matrix to subtract 2 if the result would be above 2.