ok i did according to your suggestion but somewhat it looks amatuerish ha.. cos it repeats the same thing for each percentage group: css and message. I am wondering if there is another way to change it? If not, i am ok with this..

if (69 < percentDiscount && percentDiscount < 101) {

        $(this).find("#percentoff").html('> 70% off');
        $(this).find("#percentoff").addClass('badge70');
    }

    else if (49 < percentDiscount && percentDiscount < 70) {

        $(this).find("#percentoff").html('&gt; 50% off');
        $(this).find("#percentoff").addClass('badge50');
    }

    else if (29 < percentDiscount && percentDiscount < 50) {

        $(this).find("#percentoff").html('&gt; 30% off');
        $(this).find("#percentoff").addClass('badge30');
    }


    else if (19 < percentDiscount && percentDiscount < 30) {

        $(this).find("#percentoff").html('&gt; 20% off');
        $(this).find("#percentoff").addClass('badge30');
    }

ok i did according to your suggestion but somewhat it looks amatuerish ha.. cos it repeats the same thing for each percentage group: css and message. I am wondering if there is another way to change it? If not, i am ok with this..

if (69 < percentDiscount && percentDiscount < 101) {

        $(this).find("#percentoff").html('> 70% off');
        $(this).find("#percentoff").addClass('badge70');
    }

    else if (49 < percentDiscount && percentDiscount < 70) {

        $(this).find("#percentoff").html('&gt; 50% off');
        $(this).find("#percentoff").addClass('badge50');
    }

    else if (29 < percentDiscount && percentDiscount < 50) {

        $(this).find("#percentoff").html('&gt; 30% off');
        $(this).find("#percentoff").addClass('badge30');
    }


    else if (19 < percentDiscount && percentDiscount < 30) {

        $(this).find("#percentoff").html('&gt; 20% off');
        $(this).find("#percentoff").addClass('badge30');
    }

• javascript
• jquery

Share Improve this question Follow edited Nov 18, 2010 at 4:48 joe asked Nov 16, 2010 at 10:33 joejoe 1,13555 gold badges2222 silver badges5050 bronze badges

Add a ment  | 

4 Answers 4

Sorted by: Reset to default 5

You've got the > round the wrong way. Let's look at the first one as an example:

if (percentDiscount > 69 && percentDiscount > 101) 

(I reversed the first condition because I think it makes what's next more obvious.)

So percentDiscount has to be greater than 69 AND greater than 101. You want

if (69 < percentDiscount && percentDiscount < 101)

This should do what you expect.

You're checking for a percentDiscount that's both higher than both numbers, so the in your second if check, there are no numbers left that are also above 69. It should look like this instead (keeping your exclusion logic):

if (percentDiscount > 69 && percentDiscount < 101) {    
    $(this).find("#percentoff").html('&gt; 70% off');
    $(this).find("#percentoff").addClass('badge70');
}    
else if (percentDiscount > 49 && percentDiscount < 69) {    
    $(this).find("#percentoff").html('&gt; 50% off');
    $(this).find("#percentoff").addClass('badge50');
}    
else if (percentDiscount > 29 && percentDiscount < 49) {    
    $(this).find("#percentoff").html('&gt; 30% off');
    $(this).find("#percentoff").addClass('badge30');
}

Swap the terms so they're in the same order like I have above, I think you'll find it's much easier to read. However, overall your terms exclude the 69, and 49 points specifically, so I'd change your logic to this:

if (percentDiscount > 69) {    
    $(this).find("#percentoff").html('> 70% off');
    $(this).find("#percentoff").addClass('badge70');
}    
else if (percentDiscount > 49) {    
    $(this).find("#percentoff").html('> 50% off');
    $(this).find("#percentoff").addClass('badge50');
}    
else if (percentDiscount > 29) {    
    $(this).find("#percentoff").html('> 30% off');
    $(this).find("#percentoff").addClass('badge30');
}

The first if grabs those above 69, the next those above, 49, etc...much simpler :)

In the first test, you are testing whether percentDiscount is greater than 69 and greater than 101. I think you meant this:

if (69 < percentDiscount && percentDiscount < 101) ...

The remaining tests have the same problem.

Take a look at this line:

if (69 < percentDiscount && percentDiscount > 101) {

There are two problems here. First: you're doing the parisons wrong. From this if statement, you're checking if percentDiscount is bigger than 69 and if it's bigger than 101. So, this will hold only if it's bigger than 101. What you probably wanted is this:

if (69 < percentDiscount && percentDiscount < 101) {

This can be written better as:

if (percentDiscount > 69 && percentDiscount < 101) {

But, there's still one problem. Operator precedence. The last example should be written as

if ( (percentDiscount > 69) && (percentDiscount < 101) ) {

to avoid ambiguities.