patterncsharpModerate
Simple math quiz in C#
Viewed 0 times
quizsimplemath
Problem
I am only a beginner when it comes to programming, but after reading tons of tutorials on and offline, I have been able to create this simple math quiz. It does work as intended, and I'm really just fishing for someone to yell at me for doing something glaringly bad practice in the code below.
```
using System;
namespace ForthConsoleProject
{
class MainClass
{
public static void Main(string[] args)
{
Random randomgen = new Random();
//int timer = 30; I considered a timer function but that ended up being way over my head.
int num01 = randomgen.Next(11);
int num02 = randomgen.Next(11);
int playeranswer;
int answer;
int numofquestions;
int numofquestionsleft;
int numofcorrect = 0;
Console.Write("How many questions would you like to answer? ");
numofquestions = Convert.ToInt32(Console.ReadLine());
numofquestionsleft = numofquestions;
// This is the loop which handles the actual question/answer core of the game.
// Answering a question correctly increases your score.
while (numofquestionsleft > 0){
Console.Write("What is " + num01 + " times " + num02 + "? ");
answer = num01 * num02;
playeranswer = Convert.ToInt32(Console.ReadLine());
if (answer == playeranswer)
{
Console.WriteLine(playeranswer + " is correct!");
numofcorrect++;
}
else {
Console.WriteLine(playeranswer + " is incorrect! Try again.");
}
Console.ReadKey();
numofquestionsleft--;
num01 = randomgen.Next(11);
num02 = randomgen.Next(11);
}
// Letting the user know how many answers they got right.
Console.WriteLine("You got
```
using System;
namespace ForthConsoleProject
{
class MainClass
{
public static void Main(string[] args)
{
Random randomgen = new Random();
//int timer = 30; I considered a timer function but that ended up being way over my head.
int num01 = randomgen.Next(11);
int num02 = randomgen.Next(11);
int playeranswer;
int answer;
int numofquestions;
int numofquestionsleft;
int numofcorrect = 0;
Console.Write("How many questions would you like to answer? ");
numofquestions = Convert.ToInt32(Console.ReadLine());
numofquestionsleft = numofquestions;
// This is the loop which handles the actual question/answer core of the game.
// Answering a question correctly increases your score.
while (numofquestionsleft > 0){
Console.Write("What is " + num01 + " times " + num02 + "? ");
answer = num01 * num02;
playeranswer = Convert.ToInt32(Console.ReadLine());
if (answer == playeranswer)
{
Console.WriteLine(playeranswer + " is correct!");
numofcorrect++;
}
else {
Console.WriteLine(playeranswer + " is incorrect! Try again.");
}
Console.ReadKey();
numofquestionsleft--;
num01 = randomgen.Next(11);
num02 = randomgen.Next(11);
}
// Letting the user know how many answers they got right.
Console.WriteLine("You got
Solution
This is pretty nice. I got some comments, some of which are my personal opinion (for example how to declare variables), some are accepted conventions (naming), and some are bug fixes.
Declaring variables
While it might be more performance friendly to declare all variables from the outset, it is best practice, I think, to declare variables only within the scope they're used. This is to decrease variable pollution in bigger functions. In such a simple app, it doesn't matter. But as your application gets bigger and better you don't want too many variables in scope.
From your code:
Also, for some variables you're splitting declaration (
Naming
.Net convention is to use
I also like
Using a for loop
So this :
Can be better written as:
And it is exactly the same.
Also, since
Much shorter and nicer.
Verifying input
Your code crashes when I type something that is NOT a number. Obviously if you're shipping the app (I know you're not, but humor me), you can't have that.
The trick is to use
If you want to keep asking for input until you get something valid, you can use a while loop :
Something of note: you need this little piece of code in two places. When you ask for the number of questions and when you ask for the answer. Instead of typing it twice, you should extract it into a method. You can easily do that in Visual Studio by selecting the code you want to extract into a method, pressing
I am not very imaginative so I just called it
Final result
```
class MainClass
{
public static void Main ()
{
var rand = new Random ();
Console.Write ("How many questions would you like to answer? ");
int correctCount = 0;
int questionCount = RequestInput ();
for (int i = 0; i < questionCount; i++)
{
int num01 = rand.Next (11);
int num02 = rand.Next (11);
Console.Write ($"What is {num01} times {num02} ?");
int playerAnswer = RequestInput ();
if (num01 * num02 == playerAnswer)
{
Declaring variables
While it might be more performance friendly to declare all variables from the outset, it is best practice, I think, to declare variables only within the scope they're used. This is to decrease variable pollution in bigger functions. In such a simple app, it doesn't matter. But as your application gets bigger and better you don't want too many variables in scope.
From your code:
num01, num02, playeranswer, and answer are only used within the loop. So it is better to declare then inside the loop (or you might accidentally use them outside or simply crowd your intellisense).Also, for some variables you're splitting declaration (
int numofquestions;) and assignment (numofquestions = Convert.ToInt32(Console.ReadLine());), and for other you're not. I think it is better to have declaration and assignment in one line, it is terser and more readable: (int numofquestions = Convert.ToInt32(Console.ReadLine());).Naming
.Net convention is to use
PascalCase for methods and classes and the like, and camelCase for variables. So, in your code: playeranswer becomes playerAnswer, and so on. You don't have any methods defined but you need to know the convention. It makes it easier to read your code. Again, it doesn't really matter for such a short app but for bigger apps such practices are a life saver. (A crude example, but I am sure you have seen the photo where KidsExchange reads like KidSexChange due to uniform capitalization KIDSEXCHANGE.)I also like
questionCount more than numOfQuestions. Shorter and uses clearer vocab.Using a for loop
for loops are easier for humans to process than while loops, I think. Well easier for me.So this :
int questionCount= Convert.ToInt32 (Console.ReadLine ());
int questionLeftCount = questionCount;
while (questionLeftCount > 0)
{
// .. do stuff
questionLeftCount --;
}Can be better written as:
int questionCount= Convert.ToInt32 (Console.ReadLine ());
for (int questionLeftCount = questionCount; questionLeftCount > 0; questionLeftCount--)
{
// do stuff ..
}And it is exactly the same.
Also, since
questionLeftCount is only used within the for loop definition, just use i or iter for iterator. It is standard practice. (It also standard practice to increment rather decrement). I am sure you have seen something that looks exactly like this in your readings.for (int i = 0; i < questionCount; i++)
{
// do stuff ..
}Much shorter and nicer.
Verifying input
Your code crashes when I type something that is NOT a number. Obviously if you're shipping the app (I know you're not, but humor me), you can't have that.
The trick is to use
int.TryParse to parse inputs. TryParse returns a bool to indicate whether the conversion is successful or not, and has an out variable which assigns a value to an already declared variable. The syntax is a bit weird and takes some getting used to, but it is being made nicer in C# 7.0 (not released yet).// Usage
int playerAnswer;
if (int.TryParse (Console.ReadLine (), out playerAnswer))
{
// do stuff with playerAnswer
}
else
{
// show error message
}If you want to keep asking for input until you get something valid, you can use a while loop :
int playerAnswer;
while (!int.TryParse (Console.ReadLine (), out playerAnswer))
{
Console.WriteLine ("Invalid Input. Please type a number using digits");
}Something of note: you need this little piece of code in two places. When you ask for the number of questions and when you ask for the answer. Instead of typing it twice, you should extract it into a method. You can easily do that in Visual Studio by selecting the code you want to extract into a method, pressing
Ctrl + . or the small light bulb to the side, and selecting, well, Extract Method. It does all the thinking for you and it is fantastic.I am not very imaginative so I just called it
RequestInput. static int RequestInput ()
{
int playerAnswer;
while (!int.TryParse (Console.ReadLine (), out playerAnswer))
{
Console.WriteLine ("Invalid Input. Please type a number using digits");
}
return playerAnswer;
}Final result
```
class MainClass
{
public static void Main ()
{
var rand = new Random ();
Console.Write ("How many questions would you like to answer? ");
int correctCount = 0;
int questionCount = RequestInput ();
for (int i = 0; i < questionCount; i++)
{
int num01 = rand.Next (11);
int num02 = rand.Next (11);
Console.Write ($"What is {num01} times {num02} ?");
int playerAnswer = RequestInput ();
if (num01 * num02 == playerAnswer)
{
Code Snippets
int questionCount= Convert.ToInt32 (Console.ReadLine ());
int questionLeftCount = questionCount;
while (questionLeftCount > 0)
{
// .. do stuff
questionLeftCount --;
}int questionCount= Convert.ToInt32 (Console.ReadLine ());
for (int questionLeftCount = questionCount; questionLeftCount > 0; questionLeftCount--)
{
// do stuff ..
}for (int i = 0; i < questionCount; i++)
{
// do stuff ..
}// Usage
int playerAnswer;
if (int.TryParse (Console.ReadLine (), out playerAnswer))
{
// do stuff with playerAnswer
}
else
{
// show error message
}int playerAnswer;
while (!int.TryParse (Console.ReadLine (), out playerAnswer))
{
Console.WriteLine ("Invalid Input. Please type a number using digits");
}Context
StackExchange Code Review Q#143492, answer score: 13
Revisions (0)
No revisions yet.