Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

isNaN shouldn't be considered a number #1406

Closed
gustavoguichard opened this issue Feb 14, 2016 · 12 comments
Closed

isNaN shouldn't be considered a number #1406

gustavoguichard opened this issue Feb 14, 2016 · 12 comments
Labels

Comments

@gustavoguichard
Copy link

One of the reasons I've chosen Flowtype is to get rid of NaN crazyness in my team's code.
I expect functions not to return NaN if I annotate them to return number.

screen shot 2016-02-14 at 6 17 21 pm

IMHO this should not be valid. Any reason why we should care this JS legacy in Flow?

Cheers

@avikchaudhuri
Copy link
Contributor

It's hard to track when operations might return NaN. Say we do ban NaN: number, In your example, what would you expect to happen when you do divide(0,0)? Right, we'd have to say that the return type of divide is number | NaN or something like that, then force NaN checks on all callers of divide. So the question really is, would that be a net benefit?

@vkurchatkin
Copy link
Contributor

I think numbers should stay as they are, but it would be nice to have a separate type for finite numbers (no NaN or Infinity).

Then we can define signatures of operators as following:

 + : (number, number) => number
 + : (number, finite_number) => number
 + : (finite_number, number) => number
 + : (finite_number, finite_number) => finite_number

 / : (finite_number, finite_number) => number

etc

@gustavoguichard
Copy link
Author

+1 for finite type.
That said, it'd be a nice DUX to somehow in my first image, get an alert.
It'd make more sense to get this alert due to params that I'm expecting rather than what's returning.

Elm way of handling is sort of cool:
screen shot 2016-02-16 at 9 13 54 am

@mroch
Copy link
Contributor

mroch commented Feb 18, 2016

plot twist: you'd agree Number.MAX_VALUE is finite, right?

(Number.MAX_VALUE + Number.MAX_VALUE) === Infinity // true

@dmitriz
Copy link

dmitriz commented May 18, 2016

I would strongly support to add the type finite.

To get away from the headache how exactly it should behave, I suggest to use Lodash isFinite function as Blueprint (with unavoidable minor modification, unfortunately, see below).

It has been there for ages for a good reason, even before Lodash existed, as one of the core utilities from Underscore, and Lodash still defers to it:

https://github.com/lodash/lodash/blob/master/vendor/underscore/underscore.js#L1311-L1314

// Is a given object a finite number?
_.isFinite = function(obj) {
    return !_.isSymbol(obj) && isFinite(obj) && !isNaN(parseFloat(obj));
};

Note it does not use the native Number.isFinite function
(https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isFinite),
but instead the less predictable global isFinite
(https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isFinite),
perhaps for historical reasons.

What we likely don't want is this:

isFinite('1') // true

Which also caused people some headaches:
http://stackoverflow.com/questions/5690071/why-check-for-isnan-after-isfinite

The signature is affected by several constants incl. Number.MAX_VALUE:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number

However, the resulting type depends on the value (instead of only type):

Number.isFinite(Number.MAX_VALUE + 1) // true, because of the rounding
Number.isFinite(Number.MAX_VALUE + Number.MAX_VALUE) // false

So what can we do?
I personally would stay sane and regard Number.MAX_VALUE as NOT of type finite.
That makes type checking really dead simple:

  • Any expression involving Number.MAX_VALUE, Number.MIN_VALUE, Number.MAX_SAFE_INTEGER, Number.MIN_SAFE_INTEGER is automatically NOT of type finite.

The reasoning: If you have any of those inside your code, you are at clear risk of getting infinities. So the mere reason you put type finite is that you want to be warned about those risks.

And perhaps, to avoid questions like "why is that not of type finite even when Number.isFinite says so", make a clear statement that finite is not a native type.

@dmitriz
Copy link

dmitriz commented May 18, 2016

Just to attempt some clarification. As a professional mathematician, I would like to have a proper mathematical type for real numbers. That would be clean and intuitive to anyone not familiar with implementation details.

The basic idea is simple. Mathematics clearly defines what real numbers are and what operations are permitted. Entities like Number.MAX_VALUE fail that definition. Entity like

Number.EPSILON: The smallest interval between two representable numbers

is not mathematically distinguishable from 0. Therefore, it must be treated as 0 for real-number typechecking purposes.

In other words, only mathematically unambiguous real numbers can be whitelisted. Only for them, algebra rules guarantee operational safety. All other entities may lead to potential bugs.

As example, this function should be considered safe:

function safeSquare (x: finite): finite {
    return x * x
}

The fact that it might get evaluated to Infinity at run-time by JS engine bares no difference, because that result can only be a side-effect of the current engine implementation. That may well change in the future. Or the result will depend on the engine. Or the platform. Or the OS... But people writing code know that. They don't need to be warned their argument x might get too large. After all, it is not the fault of the library maintainer if the consumer tries the library with all possible crazy values. Which includes Number.MAX_VALUE. In fact, those numbers normally have no place in your code.

But here is the real problem -- what if another programmer puts this number somewhere in the code for presumably "legitimate" purposes. Or your dependency third party library has it somewhere down the code. Then, if there is any risk, I want to be warned!

Here is a concrete example of unsafe function where I want to be warned:

function unsafeDivide (x: finite, y: finite): finite {
    return x / y
}

Here there is a clear risk that y is 0 or close to it.
On that other hand, this function would be safe:

function safeDivide (x: finite, y: nonzero): finite {
    return x / y
}

So I need the type nonzero. Or it needs to be inferred from the context.

The type nonzero signature can be again defined mathematically following the risk-free principle:

  • The result of any expression is whitelisted as having the desired type T if and only if it is guaranteed to be evaluated to the desired type T at the run-time under any circumstances.

For instance, the above function unsafeDivide cannot guarantee it, so it fails the type check. The above function safeDivide guarantees safety, so it passes.

Note that this guarantee philosophy is orthogonal to JavaScript's don't fail at any cost. The latter tells you that 1/0 === Infinity or 1/-0 === -Infinity, throw this back into your expression and hope for the best ;) And, as will all know, our hopes can be fullfilled sometimes. But sometimes they aren't. Which is why we are here ;)

Yes, it makes a perfect business sense to keep your app running suppressing all those errors that might be of no relevance to the user. Or the user will be presented with the payment bill of 1e+30 dollars. ;)
That no one forces the user to accept.

And in 98% cases, it is probably ok to move on, and the user won't even notice.
But the fact that users don't notice, or did not notice yet, does not mean it is ok for your code to keep that potential bug opportunity. Next time it might matter to user after all. In contrast, we do want to see this failure to catch that bug. Which is why we are here.

In contrast, the guarantee principle will give you as programmer a complete peace of mind, because it will warn you about any conceivable potential violation. It should not be a human task to write all imaginable unit tests to safeguard against all possible imaginable and non-imaginable troubles. Not even mention, many teams don't even bother writing tests at all or write only minimum possible. The right "person" to deal with this problem is the compiler!

@vkurchatkin
Copy link
Contributor

Any expression involving Number.MAX_VALUE, Number.MIN_VALUE, Number.MAX_SAFE_INTEGER, Number.MIN_SAFE_INTEGER is automatically NOT of type finite

This is not enough, any expression involving should not evaluate to finite as well.

would like to have a proper mathematical type for real numbers

I think it's impossible to use javascript numbers for this.

@dmitriz
Copy link

dmitriz commented May 19, 2016

This is not enough, any expression involving should not evaluate to finite as well.

Exactly what I tried to say by "NOT of type finite":

  • Any expression involving non-mathematical entities fails the finite type check.

I think it's impossible to use javascript numbers for this.

What are the problems?

@vkurchatkin
Copy link
Contributor

I meant to say, any expression involving finite. There is nothing special about Number.MAX_VALUE, you can get it by adding two finite numbers

What are the problems?

For starters, real numbers are an infinite set, javascript numbers are a finite set

@dmitriz
Copy link

dmitriz commented May 19, 2016

I meant to say, any expression involving finite. There is nothing special about Number.MAX_VALUE, you can get it by adding two finite numbers

Adding two of those numbers will give you Infinity. That is what is special.
Therefore my proposal to fail them for being finite.

What are the problems?
For starters, real numbers are an infinite set, javascript numbers are a finite set

How is this a problem for my proposal?

@vkurchatkin
Copy link
Contributor

@dmitriz

Adding two of those numbers will give you Infinity

Not true, (8.988465674311579e+307 + 8.988465674311579e+307) === Number.MAX_VALUE

How is this a problem for my proposal?

It's not a problem with your proposal, it's a problem with 'proper mathematical type for real numbers' idea

@mroch
Copy link
Contributor

mroch commented Oct 14, 2016

The fact that it might get evaluated to Infinity at run-time by JS engine bares no difference, because that result can only be a side-effect of the current engine implementation. That may well change in the future. Or the result will depend on the engine. Or the platform. Or the OS...

This behavior is part of the spec and is not implementation-dependent:
http://www.ecma-international.org/ecma-262/6.0/#sec-ecmascript-language-types-number-type

Unfortunately I don't think this is something that Flow will be able to prove.

@mroch mroch closed this as completed Oct 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants