Why is one variable greater than the other in this code?

Go To StackoverFlow.com

0

Why is rfd_total > max_rfd true? I don't understand how rfd_total can be greater than max_rfd in the following code:

max_rfd = parseFloat(jQuery('#mx-rfd_'+order_id).val()).toFixed(2);

rfd_total = parseFloat(items_total+tax_total+shipping+allowances*1).toFixed(2);


if( rfd_total > max_rfd)
{   if(isNaN(rfd_total)) alert('rfd_total isNaN'); // not triggered
    if(isNaN(max_rfd)) alert('max_rfd isNaN'); // not triggered
    alert(rfd_total); // alerts 51.16
    alert(max_rfd); // alerts 102.32
    return false;
}
2012-04-05 00:39
by WallabyKid
Get a console.log(maxrfd, rdftotal) in there. Get Chrome dev-tools or Firebug out and look at the output. You will be able to see their values and whether they are strings or numbers ; - Dan Prince 2012-04-05 00:44
for a quick test in the if statement do if( (rfdtotal *1) > (maxrfd * 1)) then it should convert to a number. That should yield the results you want. If it does then apply it the initial variables in replacement of parseFloat. EDIT: Griffin pointed out that toFixed() returns a string. So multiply by 1 after toFixed() if you wish to use that method - Mike Depies 2012-04-05 00:45
@MikeDepies as you can see I had resorted to the *1 approach in setting rfdtotal, but again, what I don't get is why neither isNaN(rfdtotal) NOR isNaN(max_rfd) return true - WallabyKid 2012-04-05 00:54
@WallabyKid I explained in the comments of my answer : - Griffin 2012-04-05 01:01
Look at Griffins answer as he mentions the toFixed issue. That is where your issue lie - Mike Depies 2012-04-05 01:11


4

It's because rfd_total an max_rfd are Strings.

You will notice that "51.16" > "102.32" returns true.

toFixed() returns a string.

You will need to coerce your variables to numbers, which you can find out how to do with a quick search.

Or you can keep your code clean and do it properly using a function such as this one

function decimalRoundTo(n, decimalPlaces) {
    var d = Math.pow(10, decimalPlaces);
    return Math.round(n*d)/d;
}
2012-04-05 00:43
by Griffin
Well there you go! News to me. That explains soooo much. I never knew toFixed() retunred a string! ... but then, wouldn't isNaN() return false - WallabyKid 2012-04-05 00:49
@WallabyKid, you're right, isNaN returns false. That is why your alerts aren't triggered. I've added a bit more to my answer btw - Griffin 2012-04-05 00:54
by return false I meant return true! e.g rfd_total isNaN because it is a string. Right - WallabyKid 2012-04-05 00:58
@WallabyKid, isNaN attempts to coerce the input parameter to a number. isNaN("12.13") returns false, but isNaN("hello") returns true - Griffin 2012-04-05 01:00
Thanks! I like the function approach cuz I'm so tired of fighting for it. I'm used to PHP which is much more intuitive regarding strings and number - WallabyKid 2012-04-05 01:03
JavaScript's weak typing can be very handy at times. But in examples like this you really don't want to be seeing string * number in your code. Glad I could help - Griffin 2012-04-05 01:07
A special function is not required. The string returned by toFixed can be converted to a number using unary "+", so +rfd_total > +max_rfd is sufficient, or if a more explicit converion is required, then something like: Number(rfd_total) > Number(max_rfd) will do - RobG 2012-04-05 02:06
@RobG did you not read the rest of my answer or the comments here? Also WallabyKid, if this solves your problem please consider marking it as accepted - Griffin 2012-04-05 11:00
@Griffin—yes, I did. My comment is in regard to your statement "Or you can keep your code clean and do it properly using a function", which is made without any justification or explanation of why it might be "proper". If such a function was needed, it would be far simpler as: function decimalRoundTo(n, places) {return Number(n.toFixed(places));}. But I don't think that's required since the code itself is no longer than the call and for accuracy's sake, numbers should only be rounded for presentation. So what's the point - RobG 2012-04-05 14:32
Ads