7
\$\begingroup\$

This is code for a Rock-Paper-Scissors game I made using some of the OOP concepts I learned in C#. I don't know if I am making the right choices but I am fairly confident. It would be great if I could receive guidance here.

I chose to have a Player and Computer class inheriting from an abstract Participant class. Everything else is control flow which is inside the Gameclass which has a struct inside it, GameInfo, which will store all information regarding global game data.

using System;

enum RockPaperScissors
{
    Rock,
    Paper,
    Scissors
}

abstract class Participant
{
    public int wins { get; set; }
    float _winRate;
    protected RockPaperScissors selection;

    protected float winRate
    {
        get
        {
            return _winRate;
        }
        set
        {
            if (value < 0 || value > 100)
            {
                throw new Exception("value cannot be less than 0 or greater than 100");
            }
            _winRate = value;
        }
    }
    public void PrintWinRate()
    {
        this.winRate = ((float)wins / Game.Info.GamesPlayed) * 100;
        string winRate = "win rate: " + this.winRate.ToString() + "%";
        Console.WriteLine(winRate.PadLeft(50));
    }

    public abstract RockPaperScissors Select();
}

class Computer : Participant
{

    public override RockPaperScissors Select()
    {
        Random rand = new Random();
        selection = (RockPaperScissors)rand.Next(0, Enum.GetValues(typeof(RockPaperScissors)).Length);
        return selection;
    }
}
class Player : Participant
{
    public override RockPaperScissors Select()
    {
        bool isValid;
        string input;

        do
        {
            Console.Write("Please enter a valid selection: ");
            input = Console.ReadLine().Trim();
            isValid = Enum.TryParse<RockPaperScissors>(input, true, out selection);
        } while (!isValid);

        return selection;
    }
}



class Game
{
    public struct GameInfo
    {
        public int GamesPlayed;
    }
    public static GameInfo Info = new GameInfo();

    static void Main()
    {
        Console.BackgroundColor = ConsoleColor.White;
        Console.ForegroundColor = ConsoleColor.Black;
        Participant comp = new Computer();
        Participant player = new Player();
        RockPaperScissors computerSelection;
        RockPaperScissors playerSelection;
        ConsoleKeyInfo input;
        bool playAgain;

        do
        {
            Console.Clear();
            computerSelection= comp.Select();
            playerSelection = player.Select();
            Console.Clear();

            Console.WriteLine("Player: " + playerSelection);
            Console.WriteLine("\n" + "Computer: " + computerSelection);

            switch (determineWinner((int)computerSelection, (int)playerSelection))
            {
                case null:
                    Console.Write("\n it's a tie");
                break;

                case true:
                    Console.Write("\n you won!");
                    player.wins++;
                    break;

                default:
                    Console.Write("\n you lost");
                    comp.wins++;
                    break;
            }

            Game.Info.GamesPlayed++;
            Console.WriteLine("\n" + "Play again? <y/n>");
            Console.WriteLine("\n");

           int resetPosY = Console.CursorTop;
           int resetPosX = Console.CursorLeft;

            Console.SetCursorPosition(30, 0);
            player.PrintWinRate();
            Console.SetCursorPosition(30, 2);
            comp.PrintWinRate();
            Console.SetCursorPosition(resetPosX, resetPosY);

            input = Console.ReadKey(true);
            playAgain = input.KeyChar == 'y';

        } while (playAgain);
    }
    public static bool? determineWinner(int playerSelection, int computerSelection)
    {

        bool?[,] winMatrix = { 
            {null, false, true },
            {true, null, false },
            {false, true, null}
        };

        if (winMatrix[playerSelection, computerSelection] == null)
            return null;
        return (winMatrix[playerSelection, computerSelection] == true) ? true : false;
    }
}
\$\endgroup\$
3
  • 1
    \$\begingroup\$ I fixed your spelling, you'll have to think of a decent title yourself. Try something descriptive. \$\endgroup\$
    – Mast
    May 21, 2016 at 20:57
  • 1
    \$\begingroup\$ With descriptive I meant something telling what your code does. Your concerns should be posted in the question, not in the title. \$\endgroup\$
    – Mast
    May 21, 2016 at 21:15
  • 1
    \$\begingroup\$ I have rolled back the last edit. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Vogel612
    May 21, 2016 at 22:11

2 Answers 2

3
\$\begingroup\$

What is the point of this struct?

public struct GameInfo
{
    public int GamesPlayed;
}

It stores one value, and nothing else. Even if you intend to add more fields to this struct, they should all just be static fields that belong to the Game class, as things like GamesPlayed make sense when they're stored in the Game class. In addition, with the struct, you're always going to have to add a Info. qualification whenever you need to access info that pertains to the game. This will begin to make your code look somewhat cluttered and ugly.

As mentioned in an answer to your previous Rock Paper Scissors question, this enum would be better named Selection:

enum RockPaperScissors
{
    Rock,
    Paper,
    Scissors
}

The reason this is better named Selection is because the enum represents exactly that - a selection. You also refer to variables/parameters/fields of the type RockPaperScissors as selections across your code as well.

Other than those two suggestions, most of the good points were made in @MatMug's excellent answer to your previous Rock Paper Scissors question.

\$\endgroup\$
6
  • 1
    \$\begingroup\$ Great advice! my only question is in what situations would you nest a struct inside a class? \$\endgroup\$ May 21, 2016 at 23:15
  • \$\begingroup\$ @funTertain Generally you shouldn't ever do something like that. Structs are supposed to be small, immutable data objects that are intended to be used in more than one place (like a class). \$\endgroup\$ May 21, 2016 at 23:16
  • \$\begingroup\$ Would it be advisable to use a struct if you wanted a Player class to look like this? Player.attribs.Agility or Player.playerInfo.Wins where you would have a Attributes and PlayerInfo struct nested within the Player class? \$\endgroup\$ May 21, 2016 at 23:19
  • \$\begingroup\$ @funTertain Are you wondering whether it would be fine to use a struct to represent a Player? If so, it depends. If you just need a container for data that won't change, then yes. If, however, you need custom behaviors that change the player's data, then no. \$\endgroup\$ May 21, 2016 at 23:20
  • 1
    \$\begingroup\$ At this point I might reiterate my own comments on FooInfo classes (TL;DR: Don't use them except when SRP actually requires a second class). \$\endgroup\$
    – Kevin
    May 22, 2016 at 3:33
1
\$\begingroup\$

Couldn't you replace

if (winMatrix[playerSelection, computerSelection] == null) return null; 
return (winMatrix[playerSelection, computerSelection] == true) ? true : false; } }

By

return (winMatrix [playerSelection, computerSelection] )

?

\$\endgroup\$
1
  • \$\begingroup\$ Yes, I figured that out later but forgot to edit it. \$\endgroup\$ May 21, 2016 at 22:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.