3

I have just started to learn C# this week and am trying to run a simple code that prompts the user to enter a number if they enter text, or prompts them to enter a positive number if they enter a negative one (so a boolean operation for the text, and an if statement for the negative). If they enter a valid (positive) number the program continues on with the rest of the steps.

However with this code, if the user inputs a negative, then a text, then another negative number and so forth it seems to break the loop and continue on with the next operations.

The code is part of a bigger program so I have scaled it down and pulled out only the most critical parts for it to run. Is anyone able to spot what I have missed here?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace IncomeTaxCalculator
{
    class IncomeTax
    {
        public static void Main()
        {
            double income;
            income = income_input();
            show_output(income);
        }
        public static double income_input()
        {
            double income; string income_string; bool bad_value = true;
            do
            {
                Console.Write("What is your total income: ");
                income_string = Console.ReadLine();
                if (double.TryParse(income_string, out income))
                {
                    bad_value = false;
                }
                else
                {
                    Console.WriteLine("Enter your income as a whole-dollar numeric figure.");
                }
                if (income < 0)
                {
                    Console.WriteLine("Your income cannot be a negative");
                }
            } while (bad_value || income < 0);
            return income;
        }
              public static void show_output(double income)
        {
            Console.WriteLine("Your income is " + income);
            Console.WriteLine("\n\n Hit Enter to exit.");
            Console.ReadLine();
        }
    }
}
david_10001
  • 492
  • 1
  • 6
  • 22

3 Answers3

4

Here's what's happening. When you enter a negative number bad_value will be set to false. Then when you enter a non-numeric value income will be set to 0 by the TryParse. Now your condition of bad_value || income < 0 is false. To fix it you just need to reset bad_value to true at the beginning of each loop.

Alternatively you could as René Vogt suggests set bad_value to true in the else and additionally in the if that checks if it is negative and then you can just do while(bad_value).

do
{
    Console.Write("What is your total income: ");
    income_string = Console.ReadLine();
    if (double.TryParse(income_string, out income))
    {
        bad_value = false;
    }
    else
    {
        Console.WriteLine("Enter your income as a whole-dollar numeric figure.");
        bad_value = true;
    }
    if (income < 0)
    {
        Console.WriteLine("Your income cannot be a negative");
        bad_value = true;
    }
} while (bad_value);
juharr
  • 31,741
  • 4
  • 58
  • 93
0

Change your code to be something like this:

 double income;
 string income_string;
 do
 {
        Console.Write("What is your total income: ");
        income_string = Console.ReadLine();
 } while (!double.TryParse(income_string, out income) || income < 0);
//rest of your code here, in another method that takes the valid income

You should split the method that procures income from the one that has (business ) logic in it.

Dido
  • 520
  • 2
  • 7
  • 21
0

I realize this has already been accepted but this can be done in a much simpiler loop. Why not just create an infinite loop and break/return when the values are satisfied. Instead of checking for invalid input search for valid input.

I wont go into detail as to why this is a more acceptable solution, consider the instructions given, if you exepect an invalid input then your instructions are wrong. Instead check for positive results. Read This!!

static double income_input()
{
    double income = double.NaN;
    while (true)
    {
        Console.WriteLine("What is your income?:");
        if (double.TryParse(Console.ReadLine(), out income) && income > 0)
            return income;
        Console.WriteLine("Invalid input. Please enter a valid number greater than zero.");
    }
}

Really all we have done here is created an with the while(true). So now the loop can never end unless we explicitly tell it to.

Next you can simply parse the result and assure the conditions that double.TryParse succeeds and income > 0. Note the return simply exits the loop.

Now this compiles (note there is no return at the end) as the compiler understands that the only exit point is through the return statement. Example Post

If you wanted to go for the shortest code possible could use some C# 7 syntax for inline variables.

static double income_input()
{
    while (true)
    {
        Console.WriteLine("What is your income?:");
        if (double.TryParse(Console.ReadLine(), out double income) && income > 0)
            return income;
        Console.WriteLine("Invalid input. Please enter a valid number greater than zero.");
    }
}

Happy Coding!.

Nico
  • 12,493
  • 5
  • 42
  • 62