First of all, I suggest extracting the tax rate calculation out into its own method. This will keep things clean and enable early returns from the method, allowing us to remove a bunch else statements. As long we check the ranges in order (either ascending or descending) we don't need to check both boundaries (ie >= x && <= y). This also makes the method testable which is something you should always consider -- the event handler for the button click isn't very testable.*
Second, I suggest you use decimal for the monetary calculations and not double. For more info on why, see this answer (by Jon Skeet) and this question (and it's duplicate). I'll note however that since this looks like a learning exercise, double should be fine. However keep in mind you wouldn't typically want to use them for monetary calculations.
Let's define our method. It will accept the status and our income. It will validate that the status is a valid value (and not say, Head Of Household) and throw an exception otherwise. Then we can process each range in order and return without needing to nest else statements:
public static decimal GetTaxRate(string status, decimal income)
{
// we need to validate we have an appropriate status
var comp = StringComparison.OrdinalIgnoreCase;
if (!string.Equals(status, "married", comp) && !string.Equals(status, "single", comp))
throw new InvalidOperationException("Invalid Status");
if (string.Equals(status, "single", comp))
{
// note lack of "else" since we early return
// note lack of " && income >= " which is implied
if (income <= 9725) return 0.1m;
if (income <= 37650) return 0.15m;
if (income <= 91150) return 0.25m;
if (income <= 190150) return 0.28m;
if (income <= 413350) return 0.33m;
if (income <= 415050) return 0.35m;
}
else // married
{
if (income <= 18550) return 0.1m;
if (income <= 75300) return 0.15m;
if (income <= 151900) return 0.25m;
if (income <= 231450) return 0.28m;
if (income <= 413350) return 0.33m;
if (income <= 415050) return 0.35m;
}
// default for both
return 0.396m;
}
Now we can call this function from your button click event handler:
string status = "single"; // get this some how
string incomeAsString = "1234.56"; // get this value some how
// validate that the income is a valid value
decimal income;
if (!decimal.TryParse(incomeAsString, out income))
{
lblresult.Text = "Entered income is not a valid decimal value";
return;
}
decimal taxRate = GetTaxRate(status, income); // calculate tax rate
decimal totalTax = taxRate * income;
lblresult.Text = string.Format("Your tax rate is {0:P2} with taxes owed of {1:C2} (on income of {2:C2})", taxRate, totalTax, income);
I would suggest looking at ways to de-stringify your status variable. You could use an enum to represent the different states. You could even use a bool isSingle if you only ever plan on having the two options (though I'm not a fan of that option).
If you wanted to use some C#8 features, we could use a recursive pattern matching switch statement instead. I'm also using here C#7 digit separators (_) to make the numbers a bit more human readable:
// warning C# 8.0+
public static decimal GetTaxRate(string status, decimal income)
{
// note: validation elided
switch (status.ToLower(), income)
{
case ("single", decimal s) when s <= 9_725:
case ("married", decimal m) when m <= 18_550:
return 0.1M;
case ("single", decimal s) when s <= 37_650:
case ("married", decimal m) when m <= 75_300:
return 0.15M;
case ("single", decimal s) when s <= 91_150:
case ("married", decimal m) when m <= 151_900:
return 0.25m;
case ("single", decimal s) when s <= 190_150:
case ("married", decimal m) when m <= 231_450:
return 0.28m;
case (_, decimal i) when i <= 413_350:
return 0.33m;
case (_, decimal i) when i <= 415_050:
return 0.35m;
default:
return 0.396m;
}
}
In C#9 we'll be able to make use of relational patterns within switch expressions (see SharpLab example)
// warning C# 9.0
public static decimal GetTaxRate(string status, decimal income)
{
// note: validation elided
if (string.Equals(status, "single", StringComparison.OrdinalIgnoreCase))
{
return income switch {
<= 9725 => 0.1M,
<= 37650 => 0.15M,
<= 91150 => 0.25M,
<= 190150 => 0.28M,
<= 413350 => 0.33M,
<= 415050 => 0.35M,
_ => 0.396M // default
};
}
// married
return income switch {
<= 18550 => 0.1M,
<= 75300 => 0.15M,
<= 151900 => 0.25M,
<= 231450 => 0.28M,
<= 413350 => 0.33M,
<= 415050 => 0.35M,
_ => 0.396M // default
};
}
Note: m and M are the suffixes used to designate a decimal literal. The casing does not matter--they are interchangable
* The event handler can be manually tested and validated (someone eyeballing the label text) but ideally you'd want to be able to use tools pole xUnit where you can define automated cases.
