If statement returns wrong value

[brycematheson]

I can't figure out why this code won't work. I'm trying to write a program where depending on the user's income, it will place them in a tax bracket. However, I can only get it to output 33% and 35%. What am I doing wrong?

#include <iostream>
using namespace std;

/*************************************************
* Computes the tax percentage and returns to main.
**************************************************/

float computeTax(float income, float bracket)
{
    if ((income >= 0.0) && (income <= 15099.0));
        bracket=10;
    if ((income >=15100.0) && (income <= 61299.0));
        bracket=15;
    if ((income >= 61300.0) && (income <= 123699.0));
        bracket=25;
    if ((income >= 123700.0) && (income <= 188449.0));
        bracket=28;
    if ((income >= 188450.0) && (income <= 336549.0));
        bracket=33;
    if ((income >= 336500.0)
        bracket=35;
    return bracket;
}

/**********************************************************************
* Asks the user for their income and then places them in a tax bracket.
***********************************************************************/
int main()
{
    //Takes the user income
    float bracket;
    float income;
    cout << "Income: ";
    cin >> income;

    //Returns tax bracket percentage
    bracket = computeTax(income, bracket);
    cout << "Your tax bracket is " << bracket << "%" << endl;
    return 0;
}

[kooth]

You have semi-colons at the end of your "if" statements.

[brycematheson]

Well that certainly was easy. Thank you very much, Kooth. As you can tell, I'm just learning. ;)

[kooth]

I'm glad I could help! I remember when I first started out learning to code -- and I remember the people who helped me, so it's my time to return the favor!

Good luck!

[Disch]

You have another very common problem:

- don't put upper and lower bounds in your if statements. It's redundant and error prone.

example... What bracket do you get if their income is 15099.5?

    if ((income >= 0.0) && (income <= 15099.0))   // not this one... 
        bracket=10;
    if ((income >=15100.0) && (income <= 61299.0))  // and not this one either
        bracket=15;

The better way to do this is to have one comparison value in the if statement, and have 'else' take care of the exclusion work:

if(income < 0)
  InvalidInput();
else if(income < 15100)
  bracket = 10;
else if(income < 61300)
  bracket = 15;
//... 

[brycematheson]

If I do it this way though, wouldn't that give me a very error prone output, also? For example, you used the code:

else if(income < 61300)
bracket = 15;

How does the computer know whether I'm in the 10% bracket or the 15% bracket? Because less than 61,300 can range anywhere from 0 to 61,300 which also protrudes on the 10% bracket range of 0 to 15,100. How does this work?

[kradreyals]

I guess the computer checks in the order that you wrote your code. So it'll check first if it's in the range 10%. Remember you are using 'else if,' which means your program will skip past all the 'else if,' once one of the 'if' is true. That's why you use 'else if,' it's tied to your first 'if' statement and only goes to the next line if it's false, unlike the current model, which runs 6 'if' at a time and the 6 have the possibility of being true.

The template is
if statement
else if statement(statement above is false)
else do this (all statements are false)

[Disch]

+1 kradreyals

The else keyword means the following will run only if the previous if statement was false.

Example:

if(foo == 5)
{
  cout << "this will print only if foo is equal to 5";
}
else
{
  cout << "this will print only if foo does not equal 5";
}

When combined with another if (else if), the following if only runs if the else runs... and the else only runs if the previous if was false. So:

if(foo < 5)
{
  cout << "foo is less than 5";
}
else if(foo < 10)
{
  cout << "the else ensures that foo is not less then 5" << endl
  cout << "and the if statement ensures that foo is less than 10" << endl;
  cout << "therefore this only runs if foo is >= 5 and < 10";
}
else
{
  cout << "foo is >= 10";
}

[wtf]

It would also help stylistically to indent your code so that you can visually tell which code is being tied to which if statement. something like:

 
if(foo < 5)
    {
          cout << "foo is less than 5";
    }
    else if(foo < 10)
        {
              cout << "the else ensures that foo is not less then 5" << endl
              cout << "and the if statement ensures that foo is less than 10" << endl;
              cout << "therefore this only runs if foo is >= 5 and < 10";
        }
    else
        {
              cout << "foo is >= 10";
        }

[Disch]

That's the first I've ever seen code indented like that, actually =P

I typically indent only for {} blocks (or for single line if/while/etc).

If I had another set of if/else's following and it needed some extra clarity, I would separate them with a comment block rather than indenting.

Exessive indenting is just as bad as not enough indenting, IMO.

I'd probably do something like this:

if(foo)
{
    Stuff();
}
else if(bar)
{
    MoreStuff();
}
else
{
    OtherStuff();
}

//=========================

if(baz)
{
    EvenMoreStuff();
}