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\$
  • 1
    \$\begingroup\$ I fixed your spelling, you'll have to think of a decent title yourself. Try something descriptive. \$\endgroup\$ – Mast May 21 '16 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 '16 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 '16 at 22:11
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\$
  • 1
    \$\begingroup\$ Great advice! my only question is in what situations would you nest a struct inside a class? \$\endgroup\$ – fun Tertain May 21 '16 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\$ – Ethan Bierlein May 21 '16 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\$ – fun Tertain May 21 '16 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\$ – Ethan Bierlein May 21 '16 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 '16 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\$
  • \$\begingroup\$ Yes, I figured that out later but forgot to edit it. \$\endgroup\$ – fun Tertain May 21 '16 at 22:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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