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 propertiesfields are constants or. Note a static readonly valuesvalue cannot be a constant, so if you have any of those they should be properties (be it public or private). If
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.