joe
joe

Reputation: 1135

Operators not working?

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('&gt; 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');
    }

Upvotes: 2

Views: 778

Answers (4)

Nick Craver
Nick Craver

Reputation: 630569

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

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

Upvotes: 3

Jon
Jon

Reputation: 16726

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.

Upvotes: 5

darioo
darioo

Reputation: 47193

Take a look at this line:

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

There are two problems here. First: you're doing the comparisons 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.

Upvotes: 2

Marcelo Cantos
Marcelo Cantos

Reputation: 185972

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.

Upvotes: 2

Related Questions