Becareful with isNaN and Type Conversion

As you may know, Javascript is a loosely typed language. It knows about data types, but it just doesn’t care too much about them. This can be both, a good and a bad thing. For this reason, you must always be careful when dealing with different data types, especially because they can be converted from one type to another automatically, which can lead to unexpected results and nasty bugs.

Today I came across such a situation. I was building a form that needs to update itself based on some values, such as number of tickets, and extra donations. When I was checking if there where any extra donation, I was first making sure that the donation value was a number, and not some other value. For that I did this:


if (isNaN(extra)) {
  extra = 0;
}

As you can see, we assume that if the value is not a number, it must be 0, meaning no donation is made. This seems like a good working code, but it is not. It has a few flaws that make it buggy.

Before continuing, you should know that the extra value is used later to calculate the total of the transaction. For that I have a line like this:

grand_total = ticket_price + extra

What happens if the user inputs 3 on the donation field, and the ticket is 100. You would expect the grand total to be 103, but it is not. It is actually 1003. This is because the 3 is a string type, not a number. There are a few ways to convert the 3 from a string to a number, one of them is using parseInt. So, lets add that to the code:


if (isNaN(extra)) {
  extra = 0;
}
extra = parseInt(extra, 10);

Now, we get the right result (103, not 1003). But remember that I told you that the code was buggy? Well, imagine now, that instead of entering a 3, the user leaves the donation field blank. What would happen? Well, grand total will now be NaN...

Why? Because when you do this:

if (isNaN(''))

The empty string is evaluated to false, which evaluates to 0, which has been converted to a number type, which of course is a number. So, now your code inside the if statement is not executed, and extra remains an empty string. But, when you pass an empty string to parseInt it returns NaN, because of course an empty string is not a number, and in this case there is no data conversion to make that empty string into a nice 0. Later when you want to add the ticket price to the extra donation, you are actually trying to add 100 to NaN, which will give you NaN as a result.

In other words, according to isNaN an empty string is a number, but according to parseInt an empty string is not a number, and this is thanks to type conversion.

The solution is quite simple, test the value return by parseInt to see if it is a number:

if (isNaN(parseInt(extra)))

This kind of bugs are easy to miss, and they can result in really bad situations. This is one of the reasons why type conversion is not so cool sometimes.