c# Simple score system

Question

I'm making a simple game. I'm trying to give the player a +1 score when he has a answer correct, but It keeps saying the same score 1. I want to have the score constantly updated when your answer is correct.

So if you have two answers correct, the score should update to 2, but it doesn't and keeps saying 1...

        start:

        Random numbergenerator = new Random ();

        int num1 = numbergenerator.Next(1,11);
        int num2 = numbergenerator.Next(1,11);
        int score = 0; // THIS IS THE SCORE

        Console.WriteLine("Whats " + num1 + " times " + num2 + "?");

        var answer = Convert.ToInt32(Console.ReadLine());

        if ( answer == num1 * num2) {
            Console.ForegroundColor = ConsoleColor.Green;
            Console.WriteLine("Thats the correct answer!");
            Console.ResetColor();
            ++score; // Gives score
            Console.WriteLine("Your score: " + score);
        } else {
            Console.ForegroundColor = ConsoleColor.Red;
            Console.WriteLine("Bummer, try again!");
            Console.ResetColor();
            ++score; // Gives score
            Console.WriteLine("Your score: " + score);
        }
        goto start;
    }
}

}

Solution 1

1) From C# specification (MSDN)

Furthermore, a variable initializer in a local variable declaration corresponds exactly to an assignment statement that is inserted immediately after the declaration.

In other words every time your program goes to start assignment statement will init score with 0

i suggest to move the initialization before start (it will be better to move numbergenerator initialization too. ) :

    Random numbergenerator = new Random ();
    int score = 0; // THIS IS THE SCORE

    start:

    int num1 = numbergenerator.Next(1,11);
    int num2 = numbergenerator.Next(1,11);

    Console.WriteLine("Whats " + num1 + " times " + num2 + "?");

    var answer = Convert.ToInt32(Console.ReadLine());

    if ( answer == num1 * num2) {
        Console.ForegroundColor = ConsoleColor.Green;
        Console.WriteLine("Thats the correct answer!");
        Console.ResetColor();
        ++score; // Gives score
        Console.WriteLine("Your score: " + score);
    } else {
        Console.ForegroundColor = ConsoleColor.Red;
        Console.WriteLine("Bummer, try again!");
        Console.ResetColor();
        ++score; // Gives score
        Console.WriteLine("Your score: " + score);
    }
    goto start;

2) Don't use goto. Because

• it makes the code confusing
• reduces readability

this article about GO TO can be interesting for you.

i suggest to replace start with while (true){ goto start; with } it should looks something like this:

    Random numbergenerator = new Random ();
    int score = 0; // THIS IS THE SCORE

    while(true)
    {
        int num1 = numbergenerator.Next(1,11);
        int num2 = numbergenerator.Next(1,11);

        Console.WriteLine("Whats " + num1 + " times " + num2 + "?");

        var answer = Convert.ToInt32(Console.ReadLine());

        if ( answer == num1 * num2) {
            Console.ForegroundColor = ConsoleColor.Green;
            Console.WriteLine("Thats the correct answer!");
            Console.ResetColor();
            ++score; // Gives score
            Console.WriteLine("Your score: " + score);
        } else {
            Console.ForegroundColor = ConsoleColor.Red;
            Console.WriteLine("Bummer, try again!");
            Console.ResetColor();
            ++score; // Gives score
            Console.WriteLine("Your score: " + score);
        }
    }

3) Extract method, it is not critical, but i suggest to improve readability by extracting a method. This method will contain loop body. The code should looks something like this:

public void TryToGuessMultiplication_GameStep(int num1, int num2)
{
            Console.WriteLine("Whats " + num1 + " times " + num2 + "?");

            var answer = Convert.ToInt32(Console.ReadLine());

            if ( answer == num1 * num2) {
                Console.ForegroundColor = ConsoleColor.Green;
                Console.WriteLine("Thats the correct answer!");
                Console.ResetColor();
                ++score; // Gives score
                Console.WriteLine("Your score: " + score);
            } else {
                Console.ForegroundColor = ConsoleColor.Red;
                Console.WriteLine("Bummer, try again!");
                Console.ResetColor();
                ++score; // Gives score
                Console.WriteLine("Your score: " + score);
            }
}

...

Random numbergenerator = new Random ();
int score = 0; // THIS IS THE SCORE

while(true)
{
    int num1 = numbergenerator.Next(1,11);
    int num2 = numbergenerator.Next(1,11);
    TryToGuessMultiplication_GameStep(int num1, int num2);      
}

4) Your code contains bug If you want to increase score only if aswer is correct you should remove ++score from else block.

5) Do not duplicate the code As you can see the last 2 statements in both if block and else block are the same. I suggest to move them out from if else operator:

public void TryToGuessMultiplication_GameStep(int num1, int num2)
{
            Console.WriteLine("Whats " + num1 + " times " + num2 + "?");

            var answer = Convert.ToInt32(Console.ReadLine());

            if ( answer == num1 * num2) {
                Console.ForegroundColor = ConsoleColor.Green;
                Console.WriteLine("Thats the correct answer!");
                ++score; // Gives score
            } else {
                Console.ForegroundColor = ConsoleColor.Red;
                Console.WriteLine("Bummer, try again!");
            }
            Console.ResetColor();
            Console.WriteLine("Your score: " + score);
}

It is not all. Now you can see that

            Console.ForegroundColor = ConsoleColor.Green;
            Console.WriteLine("Thats the correct answer!");

is quite similar (but not the same!!!) to

            Console.ForegroundColor = ConsoleColor.Red;
            Console.WriteLine("Bummer, try again!");

and we can't improve our code by moving them out of if else operator. But there is a trick - we can extract method with help of which we will reduce lines of code:

public void PrintUserMessage(ConsoleColor color, string message)
{
        Console.ForegroundColor = color;
        Console.WriteLine(message);
}

public void TryToGuessMultiplication_GameStep(int num1, int num2)
{
            Console.WriteLine("Whats " + num1 + " times " + num2 + "?");

            var answer = Convert.ToInt32(Console.ReadLine());

            if ( answer == num1 * num2){
                PrintUserMessage( ConsoleColor.Green, "Thats the correct answer!");
                ++score; // Gives score
            }else
                PrintUserMessage( ConsoleColor.Red,"Bummer, try again!");

            Console.ResetColor();
            Console.WriteLine("Your score: " + score);
}

Solution 2

Your score is set to 0 after every jump goto start;. Put your score declaration above start: like below:

 int score = 0; // THIS IS THE SCORE
 start:
 ...other instructions

Remember, no matter if you are using goto or while, if variable must store state during a loop, declare and initialize it outside that loop.

Solution 3

You're setting score to 0 every time you goto start.

Can you please stop using goto? It hurts my eyes, and I believe many other eyes as well. Use while instead.

If you use while by the way, the problem will disappear as score will not be reset every iteration.

Solution 4

1. Don't use goto it is bad practice, you should use loop instead.

Your code should look like:

        Random numbergenerator = new Random();

        int score = 0; // THIS IS THE SCORE

        while (true)
        {
            int num1 = numbergenerator.Next(1, 11);
            int num2 = numbergenerator.Next(1, 11);

            Console.WriteLine("Whats " + num1 + " times " + num2 + "?");

            var answer = Convert.ToInt32(Console.ReadLine());

            if (answer == num1 * num2)
            {
                Console.ForegroundColor = ConsoleColor.Green;
                Console.WriteLine("Thats the correct answer!");
                Console.ResetColor();
                ++score; // Gives score
                Console.WriteLine("Your score: " + score);
            }
            else
            {
                Console.ForegroundColor = ConsoleColor.Red;
                Console.WriteLine("Bummer, try again!");
                Console.ResetColor();
                Console.WriteLine("Your score: " + score);
            }
        }

View more solutions

Comments

[BJ Myers]

You initialize score to 0 every time through the program. This is a great opportunity to learn how to use the debugger. Also look into for and while loops instead of using goto.

[Steve]

Besides, if you increment score also when one gives the incorrect answer then...

[Raul Fernandez]

Ok thanks BJ Myers

[Travis J]

goto should be limited to use by the compiler only. Or at least, that is the guidance given.

[stephen.vakil]

One minor note: score should not be incremented if they get a wrong answer, probably.

[sstan]

Personally, I would have refrained from posting a full block of code, because now you'll be criticized for all the other (potential) mistakes in the code that aren't directly related to the question :)

[emilpytka]

Edited. About infinite loop, this is what author wanted isn't it? :)

[Steve]

@sstan from a certain point of view you are correct but I prefer when someone analyzes the problem and give a complete answer to the problem (also if there is something not clear to the OP). For example that Convert.ToInt32 is another big failure point to avoid