can't find error in this javascript code

Question

it's not even running. Something is wrong with the do while loop. I'm assuming it's something with all the if statements? If someone could help me out i'd greatly appreciate it. Thanks.

        do {
        entry = prompt("Enter taxable income as a valid number\n" +
                       "Or enter 99999 to end entries", 99999);
        entry = parseInt(entry);

        // calculate the tax owed here
        if (entry < 0){
            document.write("Please enter a positive number");
        }
        if (entry > 0 && entry < 8701){
            tax_owed = entry * 0.10;
        }
        if (entry > 8700 && entry < 35351){
            tax_owed = 870 * 0.15;
            tax_owed += entry;
        }
        if (entry > 35350 && entry < 85651){
            tax_owed = 4867 * 0.25;
            tax_owed += entry;
        }
        if (entry > 85650 && < 178651){
            tax_owed = 17442 * 0.28;
            tax_owed += entry;
        }
        if (entry > 178650 && entry < 388351){
            tax_owed = 43482 * 0.33;
            tax_owed += entry;
        }
        if (entry > 388350){
            tax_owed = 112683 * 0.35;
            tax_owed += entry;
        }

        alert("Tax owed is " + tax_owed);
    }
    while (entry != 99999);

What are those magic numbers? Why not assign them to variables? By refactoring your code you'll probably find the error. — elclanrs
– elclanrs, Commented Jul 21, 2014 at 8:20

Mritunjay · Accepted Answer · 2014-07-21 08:22:47Z

1

In your 21 line

if (entry > 85650 && < 178651){ // there is no operand between && and <.

I think it should be

if (entry > 85650 && entry< 178651){

answered Jul 21, 2014 at 8:22

Mritunjay

25.9k7 gold badges57 silver badges71 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Georgi-it · Accepted Answer · 2014-07-21 08:22:56Z

1

On the line if (entry > 85650 && < 178651) if you look closely will see that you are not comparing the second number to anything. It should be if (entry > 85650 && entry < 178651)

answered Jul 21, 2014 at 8:22

Georgi-it

3,6861 gold badge22 silver badges25 bronze badges

Comments

DenisFLASH · Accepted Answer · 2014-07-23 09:35:57Z

0

The error of missing operand before < 178651 is already found, but in case you want your code to have less potential errors, i suggest a light refactoring procedure.

Using else if will be more efficient. Once found a block that corresponds to a given entry value, the program will stop checking other conditions. Moreover, you will have to specify just those values which divides your range (your "milestones"), instead of repeating them like entry < 8701 followed by entry > 8700. Finally, it wil fix other bug which is printing tax_owed = ..something like 140000.. when you enter 99999.
Changing the condition of the loop (introducing a boolean variable) would be more readable.
I'm not a tax man at all, but are you sure you want to add tax_owed += entry; ? I mean, do i really owe $10130.5 when declaring $10000? In any case, it will be more readable to have 1 line instead of 2: tax_owed = entry + 17442 * 0.28; // but do you really add 'entry'?.

So, the code will be something like this:

continueEntering = true;

while ( continueEntering ) {
    tax_owed = 0;
    entry = prompt("Enter taxable income as a valid number\n" + "Or enter 99999 to end entries", 99999);
        entry = parseInt(entry);

    if (entry == 99999) {
        continueEntering = false;
    } else if (entry < 0){
        alert("Please enter a positive number"); 
    } else if (entry <= 8700){
        tax_owed = entry * 0.10;
    } else if (entry <= 35350){
        tax_owed = 870 * 0.15 + entry;
    } else if (entry <= 85650){
        tax_owed = 4867 * 0.25 + entry;
    } else if (entry <= 178650){
        tax_owed = 17442 * 0.28 + entry;
    } else if (entry <= 388350){
        tax_owed = 43482 * 0.33 + entry;
    } else {
        tax_owed = 112683 * 0.35 + entry;
    }

    alert("Tax owed is " + tax_owed);
}

As said in comments, it's really better not to use the values themselves inside the body of your loop. It will be way easier to change one day those values, if they are stored in variables. here you can use 2 arrays. You can call them, say, thresholds[] and tax_percentage[], or whatever (you know it better than me). The good thing of using arrays is that you'll be able to replace the sequence of if statements by just one for loop inside your original while loop.

================ UPDATE ====================

Here is how you can refactor the above code to use a for loop instead of lots of if.

continueEntering = true;

while ( continueEntering ) {
    tax_owed = 0;
    exitValue = 99999;

    tax_threshold = [ 0, 8700, 35350, 85650, 178650, 388350 ];
    tax_rate = [ 0.10, 0.15, 0.25, 0.28, 0.33, 0.35 ];
    additional_tax = [ 0, 870, 4867, 17442, 43482, 112683 ];

    entry = prompt("Enter taxable income as a valid number\n" + 
        "Or enter " + exitValue + " to end entries", exitValue);
    entry = parseInt(entry);

    if (entry == exitValue) {
        continueEntering = false;
    } else if (entry < 0){
        alert("Please enter a positive number"); 
    } else {

        for( i = tax_threshold.length-1; i >= 0; i-- ) {
            if ( entry > tax_threshold[i] ) {
                tax_owed = entry - tax_threshold[i];
                tax_owed *= tax_rate[i];
                tax_owed += additional_tax[i];
                break;
            }
        }
    }

    alert("Tax owed is " + tax_owed.toFixed(2));
}

edited Jul 23, 2014 at 9:35

answered Jul 21, 2014 at 9:30

DenisFLASH

7521 gold badge9 silver badges15 bronze badges

14 Comments

user3733575 Over a year ago

that looks a lot better. I have basic understandings of arrays and for loops but didnt even think about using them in this case. I am having a little trouble understanding this part of the for loop, though i = tax_threshold.length-1; i >= 0; i-- I know how to read it Im just not sure how its applying. Why i-- and not i++? And what does the -1 do at the end of tax_threshold.length do?

DenisFLASH Over a year ago

@user3733575 , good questions. Well, -1 is just because when your array a has 6 elements, a.length is equal to 6, whereas to access the array elements, you need to start from 0: a[0] for the first element, then a[1], a[2], a[3], a[4], a[5]. That's why we use length-1 for the last element.

DenisFLASH Over a year ago

Now, for( i = tax_threshold.length-1; i >= 0; i-- ) means that we simply reverse the order of the loop: we start from the i=5, and go down to i=0. Why? It is more convenient in this particular tax problem. You see, you are trying to locate entry to one of the ranges. The upper limit of the range N is the lower limit for the range N+1. But what about the last range? It doesn't have an upper limit! So, we can represent all 6 ranges as: [more than 0], [more than 8700], ... , [more than 388350].

user3733575 Over a year ago

ok I got everything you were saying except this part: "The upper limit of the range N is the lower limit for the range N+1. But what about the last range? It doesn't have an upper limit! So, we can represent all 6 ranges as: [more than 0], [more than 8700], ... , [more than 388350]." thanks for your help

user3733575 Over a year ago

oh wait i think i understand now. Since the entry needs to be between 2 numbers, you have to start from highest and work your way down. Right?

|

user3733575 · Accepted Answer · 2014-07-22 01:20:47Z

0

Ok I figured out the proper to way calculate the taxes. You guys were right. I was way off. I'm using the 2012 income tax policy because that's what my professor wanted us to use. Thanks everyone for the help again. I'm really liking this site! Great for us n00bs.

    var tax_owed = 0;
    var entry;

    continueEntering = true;

    while (continueEntering) {
        tax_owed = 0;
        entry = prompt("Enter taxable income as a valid number\n" + "Or enter 99999 to end entries", 99999);
        entry = parseInt(entry);

        if (entry == 99999) {
            continueEntering = false;
        } else if (entry < 0){
            alert("Please enter a positive number"); 
        } else if (entry <= 8700){
            tax_owed = entry * 0.10;
        } else if (entry <= 35350){
            tax_owed = entry - 8700;
            tax_owed *= 0.15;
            tax_owed += 870;
        } else if (entry <= 85650){
            tax_owed = entry - 35350;
            tax_owed *= 0.25;
            tax_owed += 4867;
        } else if (entry <= 178650){
            tax_owed = entry - 85650;
            tax_owed *= 0.28;
            tax_owed += 17442;
        } else if (entry <= 388350){
            tax_owed = entry - 178650;
            tax_owed *= 0.33;
            tax_owed += 43482;
        } else if (entry > 388350){
            tax_owed = entry - 388350;
            tax_owed *= 0.35;
            tax_owed += 112683;
        }

        alert("Tax owed is " + tax_owed.toFixed(2));

}

answered Jul 22, 2014 at 1:20

user3733575

531 silver badge7 bronze badges

2 Comments

DenisFLASH Over a year ago

As you can see, in else if (entry <= 8700) block you have only 1 line, while the other 5 cases have 3 lines of tax calculation in them. But what if we try to make that first block look like others? From your code we see, that you first substract the value of the previous "threshold" (thus making it appear 2 times in the code, which is dangerous source of bugs. But what is the threshold before the 1st block, before 8700? Well it is 0! So, we can add tax_owed = entry - 0 there, which will allow you to re-write the next line as tax_owed *= 0.10 (tax_owed was equal to entry).

DenisFLASH Over a year ago

Finally, you can add tax_owed += 0 which will not change its value, but from now on all of your 6 blocks have exactly the same format! It means that we can create 3 arrays by 6 elements and use for instead of 6 if blocks. Just check out how EASY and SAFE (no bugs) it is, from now on, to change some values or to add/remove some "tax thresholds"! Hope you've got it. See my main answer (accepted one) for a new code added in the end.

Collectives™ on Stack Overflow

can't find error in this javascript code

4 Answers 4

Comments

Comments

14 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

14 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related