Synik
Synik

Reputation: 141

If else statement is being skipped, what am i missing?

So disclaimer: C# noob here. I searched around for similar instances of code, but I couldn't figure out how to related it to my if statement. I feel like I'm missing something really obvious, but I cant figure it out. If i had to guess its probably bracket placement somewhere. Anyways, here is the statement:

if (netIncome < 30000)
{
    federalTaxRate = 0;
}
else if (netIncome == 30000 && netIncome < 60000)
{
    federalTaxRate = .1;
}
else if (netIncome == 60000 && netIncome < 100000)
{
    federalTaxRate = .2;
}
else if (netIncome == 100000 && netIncome < 250000)
{
    federalTaxRate = .3;
}
else if (netIncome >= 250000)
{
    federalTaxRate = .4;
}

I appreciate any help!

Upvotes: 1

Views: 654

Answers (2)

Pac0
Pac0

Reputation: 23174

In addition to the confusion already pointed out by Jonathan Wood (in the second check, you want your income to be between 30000 and 60000, but in the if condition you write you want it to be equal to 30000), this kind of check for "treshold levels" can be done more simply and more safely this way :

    if (netIncome < 30_000)
    {
        federalTaxRate = 0;
    }
    else if (netIncome < 60_000)
    {
        federalTaxRate = .1;
    }
    else if (netIncome < 100_000)
    {
        federalTaxRate = .2;
    }
    else if (netIncome < 250_000)
    {
        federalTaxRate = .3;
    }
    else //  (netIncome >= 250000)
    {
        federalTaxRate = .4;
    }

In this way, the checks are exactly the same (because you already checked in the first case if income was less than 30000, you don't need to recheck it to be greater than 30000 in the second case).

And using this method, we developpers can now nicely concentrate on looking at the actual important values (the tresholds values : 30000, 60000, etc.. ), and those are not written more than once. (DRY principle)

For instance, imagine tomorrow the federal treshold for the 0 tax rate changes from 30000 to 31000. If you change one of the 30000 in your code, but forgot to change the other, you now have a nasty logical bug!

EDIT: and also, it's starting to be a lot of zeros. You can use _ as ignored separator to write big numbers without having to count the zeroes 10 times to be sure, like I did ;)

Upvotes: 2

Jonathan Wood
Jonathan Wood

Reputation: 67223

Shouldn't this:

else if (netIncome == 30000 && netIncome < 60000)

Instead be:

else if (netIncome >= 30000 && netIncome < 60000)

Since the && operator requires both conditions to be true, the expression will only be true when netIncome is equal to 30,000.

Also, since the previous if statement confirmed the value is not less than 30,000, the expression could be shortened to.

else if (netIncome < 60000)

Upvotes: 6

Related Questions