HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Simple math quiz in C#

Submitted by: @import:stackexchange-codereview··
0
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

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: 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.