Skip to main content
1 of 2
Rick Davin
  • 6.4k
  • 1
  • 19
  • 31

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 properties are constants or static readonly values. 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.

Rick Davin
  • 6.4k
  • 1
  • 19
  • 31