Mixing
RockPaperScissor Game
Clash Royale CLAN TAG#URR8PPP
up vote
3
down vote
favorite
I have been studying C#, and I have made a simple math quiz program.
Is my program reasonable or is there anything I need to improve?
1) user input around
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();
c# beginner console rock-paper-scissors
edited 18 mins ago
t3chb0t
32.4k64399
asked 2 hours ago
user10443653
261
New contributor
user10443653 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
add a comment |Â
up vote
3
down vote
favorite
I have been studying C#, and I have made a simple math quiz program.
Is my program reasonable or is there anything I need to improve?
1) user input around
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();
c# beginner console rock-paper-scissors
edited 18 mins ago
t3chb0t
32.4k64399
asked 2 hours ago
user10443653
261
New contributor
user10443653 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
add a comment |Â
up vote
3
down vote
favorite
up vote
3
down vote
favorite
I have been studying C#, and I have made a simple math quiz program.
Is my program reasonable or is there anything I need to improve?
1) user input around
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();
c# beginner console rock-paper-scissors
edited 18 mins ago
t3chb0t
32.4k64399
asked 2 hours ago
user10443653
261
New contributor
user10443653 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
I have been studying C#, and I have made a simple math quiz program.
Is my program reasonable or is there anything I need to improve?
1) user input around
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();
c# beginner console rock-paper-scissors
c# beginner console rock-paper-scissors
edited 18 mins ago
t3chb0t
32.4k64399
asked 2 hours ago
user10443653
261
New contributor
user10443653 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
edited 18 mins ago
t3chb0t
32.4k64399
asked 2 hours ago
user10443653
261
New contributor
user10443653 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
edited 18 mins ago
t3chb0t
32.4k64399
edited 18 mins ago
t3chb0t
32.4k64399
edited 18 mins ago
t3chb0t
32.4k64399
32.4k64399
asked 2 hours ago
user10443653
261
New contributor
user10443653 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
asked 2 hours ago
user10443653
261
asked 2 hours ago
user10443653
261
261
New contributor
user10443653 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
New contributor
user10443653 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
user10443653 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
add a comment |Â
add a comment |Â
2 Answers
2
active
oldest
votes
up vote
1
down vote
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.
answered 32 mins ago
nalka
462
New contributor
nalka is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
add a comment |Â
up vote
1
down vote
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.
answered 46 mins ago
Rick Davin
2,992718
add a comment |Â
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
1
down vote
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.
answered 32 mins ago
nalka
462
New contributor
nalka is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
add a comment |Â
up vote
1
down vote
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.
answered 32 mins ago
nalka
462
New contributor
nalka is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
add a comment |Â
up vote
1
down vote
up vote
1
down vote
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.
answered 32 mins ago
nalka
462
New contributor
nalka is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
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.
answered 32 mins ago
nalka
462
New contributor
nalka is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
answered 32 mins ago
nalka
462
New contributor
nalka is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
answered 32 mins ago
nalka
462
answered 32 mins ago
nalka
462
462
New contributor
nalka is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
New contributor
nalka is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
nalka is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
add a comment |Â
add a comment |Â
up vote
1
down vote
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.
answered 46 mins ago
Rick Davin
2,992718
add a comment |Â
up vote
1
down vote
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.
answered 46 mins ago
Rick Davin
2,992718
add a comment |Â
up vote
1
down vote
up vote
1
down vote
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.
answered 46 mins ago
Rick Davin
2,992718
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.
answered 46 mins ago
Rick Davin
2,992718
edited 25 mins ago
answered 46 mins ago
Rick Davin
2,992718
answered 46 mins ago
Rick Davin
2,992718
answered 46 mins ago
Rick Davin
2,992718
2,992718
add a comment |Â
add a comment |Â
user10443653 is a new contributor. Be nice, and check out our Code of Conduct.
user10443653 is a new contributor. Be nice, and check out our Code of Conduct.
user10443653 is a new contributor. Be nice, and check out our Code of Conduct.
user10443653 is a new contributor. Be nice, and check out our Code of Conduct.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f205002%2frockpaperscissor-game%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
