CODEWITHSUNDEEP
c#
Raul Fernandez
Raul Fernandez

Reputation: 41

c# Simple score system

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;
    }
}

}

Upvotes: 1

Views: 14122

Answers (5)

Maxim Kitsenko
Maxim Kitsenko

Reputation: 2333

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);
}

Upvotes: 2

emilpytka
emilpytka

Reputation: 773

  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);
            }
        }

Upvotes: 1

diliad
diliad

Reputation: 1

Adding to the other answers, since you only want to increase the score when the user gets the correct answer, you should remove the ++score; line from your else statement, as the way you currently have it, the score will increase either way, regardless if the answer is correct or not.

Upvotes: 0

Paweł Dyl
Paweł Dyl

Reputation: 9143

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.

Upvotes: 2

zmbq
zmbq

Reputation: 39069

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.

Upvotes: 1

Related Questions