I have been studying C# for 2 weeks already, I wrote a simple math game but not quite sure how bad it is. I'm just teaching myself to code through internet references. If you could review my work, I would appreciate it.
- It is a simple math quiz game
- player gets 1 point in every correct answer
- the game will be over when the player gives the incorrect answer
- it displays total score the player earned, then the score resets.
- Is my program reasonable or is there anything i need to improve?
static void Main(string[] args)
{
Console.Write("Please enter your Name: ");
string userName = (Console.ReadLine());
Console.WriteLine("Hello " + userName + ", Press ENTER to start the Math Quiz");
Console.WriteLine();
Console.ReadKey();
Start:
Random numberGenerator = new Random();
int score = 0;
while (true)
{
int num01 = numberGenerator.Next(1, 11);
int num02 = numberGenerator.Next(1, 11);
Console.WriteLine("What is " + num01 + " times " + num02 + " equal to?");
int Answer = Convert.ToInt32(Console.ReadLine());
int correctAnswer = num01 * num02;
if (Answer == num01 * num02)
{
Console.ForegroundColor = ConsoleColor.Green;
++score;
int responseIndex = numberGenerator.Next(1, 5);
switch (responseIndex)
{
case 1:
Console.WriteLine("Great!");
Console.WriteLine("Your score: " + score);
break;
case 2:
Console.WriteLine("You nailed it!");
Console.WriteLine("Your score: " + score);
break;
case 3:
Console.WriteLine("You're correct!");
Console.WriteLine("Your score: " + score);
break;
default:
Console.WriteLine("Good Job " + userName + ", Keep it up!");
Console.WriteLine("Your score: " + score);
break;
}
Console.ResetColor();
Console.ReadLine();
Console.WriteLine();
}
else
{
Console.ForegroundColor = ConsoleColor.Red;
int responseIndex2 = numberGenerator.Next(1, 5);
switch (responseIndex2)
{
case 1:
Console.WriteLine("Are you even trying? The correct answer is " + correctAnswer);
break;
case 2:
Console.WriteLine("Ooops!!! The correct answer is " + correctAnswer);
break;
case 3:
Console.WriteLine("Oh, come on " + userName + " I know you can do better than that! The correct answer is " + correctAnswer);
break;
default:
Console.WriteLine("Sorry " + userName + ", that's incorrect, the correct answer is " + correctAnswer);
break;
}
Console.WriteLine(Environment.NewLine + "Game Over, Your score: " + score);
Console.ResetColor();
Console.WriteLine();
Console.ReadLine();
goto Start;
}
}
}
2 Answers 2
Even though this is a fairly short program, resist the temptation to do everything in the Program
class of your console app. Whenever I start a new console app, the first thing I do is create a Runner
class with an Execute()
method and call that from the Main()
of Program
.
Using goto
is IMHO incredibly rare in a C#/.NET program. Make this code block a method and call it.
Whenever you start copy-pasting code, it's a sign you're doing something wrong.
switch (responseIndex)
{
case 1:
Console.WriteLine("Great!");
Console.WriteLine("Your score: " + score);
break;
case 2:
Console.WriteLine("You nailed it!");
Console.WriteLine("Your score: " + score);
break;
case 3:
Console.WriteLine("You're correct!");
Console.WriteLine("Your score: " + score);
break;
default:
Console.WriteLine("Good Job " + userName + ", Keep it up!");
Console.WriteLine("Your score: " + score);
break;
}
Clearly Console.WriteLine("Your score: " + score);
should be in a method of its own, and the other line you be as well, with its message the parameter for that method, e.g.
private void ReportResult(string message, int score)
{
Console.WriteLine(message);
Console.WriteLine("Your score: " + score);
}
You could even write the second line as Console.WriteLine("Your score: {0}", score);
or Console.WriteLine($"Your score: {score}");
. While a small amount of string concatenation isn't a performance killer, newer versions of C# offer nicer and more readable ways to format strings.
int Answer
: "Answer" should be camelCase.
Considering that you calculate int correctAnswer = num01 * num02;
, why then do you do if (Answer == num01 * num02)
next? Use correctAnswer
.
Even though the current code is fairly short, I'd still move the code in the if
and the code in the else
block to a method of their own (e.g. "ReportSuccess" and "ReportFailure"). This will make your code easier to follow and splits up the logic into smaller chunks.
-
\$\begingroup\$ Hi sir, thank you so much. I will rewrite the program and do my best to improve it. One problem i find in coding is when to start and close a method, quite bit confuse about it, somehow I learned something about from your tips, thanks a lot. \$\endgroup\$Evan Michael Jore– Evan Michael Jore2018年08月27日 15:01:58 +00:00Commented Aug 27, 2018 at 15:01
-
\$\begingroup\$ Hi sir, I already made some revisions on my work, well, not much, haven't done the new block as recommend, haven't go through method yet. Back to my problem, I already eliminated the goto as advised, however, my scoring seems not working right, the condition should be every time the game resets the score will reset as well, in my case, when the game is over, the user was given 2 options to continue or to quit, say if the user choose to continue the game will reset, however the score wont revert back to 0. Appreciate if you could help me. \$\endgroup\$Evan Michael Jore– Evan Michael Jore2018年08月31日 07:13:11 +00:00Commented Aug 31, 2018 at 7:13
-
\$\begingroup\$ @EvanMichaelJore See my comment to your post WRT your update. Also note that broken code is off-topic for this site. Please follow the tour and read "How do I ask a good question?", "What topics can I ask about here?" and "What types of questions should I avoid asking?". \$\endgroup\$BCdotWEB– BCdotWEB2018年08月31日 09:06:40 +00:00Commented Aug 31, 2018 at 9:06
Since the random number to select your response is already called responseIndex
, you may as well use it as an index.
Your code
int responseIndex = numberGenerator.Next(1, 5);
switch (responseIndex)
{
case 1:
Console.WriteLine("Great!");
Console.WriteLine("Your score: " + score);
break;
case 2:
Console.WriteLine("You nailed it!");
Console.WriteLine("Your score: " + score);
break;
case 3:
Console.WriteLine("You're correct!");
Console.WriteLine("Your score: " + score);
break;
default:
Console.WriteLine("Good Job " + userName + ", Keep it up!");
Console.WriteLine("Your score: " + score);
break;
}
becomes
int responseIndex = numberGenerator.Next(0, 4);
string[] messages = { "Great!", "You nailed it!", "You're correct!", "Good Job " + userName + ", Keep it up!" };
Console.WriteLine(messages[responseIndex]);
Console.WriteLine("Your score: " + score);
goto
? In C#, (possibly) the only sensible use for thegoto
statement is to imitate the fallthrough inswitch
statements. \$\endgroup\$