3 edge cases and non-normalized Decimals

[jakub791]

edge cases

There are 3 edge cases with number input to new Decimal/Decimal.fronNumber: Infinity, -Infinity and NaN. To see why, try this code:

new Decimal(Infinity).eq(Decimal.dInf);
new Decimal(-Infinity).eq(Decimal.dNegInf);
new Decimal(NaN).new(Decimal.dNaN);

You probably expect all 3 statements to be true. Unfortunely, they all evaluate to false. Why? The fromNumber Decimal method sets layer to 0 no matter what. After all fields are set, we normalize. But the normaluze method only increments layer once if mag is above 9e15 - even if Decimal's mag was Infinity/-Infinity/NaN. So we end up with Decimals with layer 1 - while Decimal.dInfhas layer Infinity, Decimal.dNegInf has layer -Infinity and Decimal.dNaN has layer NaN. Let's try something else:

new Decimal(1e100).gt(NaN);
new Decimal("1e500").gt(Infinity);

Most users would probably expect these to be false - honestly at first I would too. Unfortunely, these are both true. Again, because Infinity/-Infinity/NaN isn't properly handled in the fromNumber method. I can think of 3 solutions: 1.Tell users to use Decimal.dInf, Decimal.dNegInf and Decimal.dNaN constants. 2.handle Infinity/-Infinity/NaN properly in the fromNumber method. 3.probably the worst option, change all methods to be aware of Decimals like {sign: 1, mag: Infinity, layer: 0} created by passing Infinity/-Infinity/NaN as input to new Decimal.

non-normalized Decimals

The Decimal.fromComponents_noNormalize method can be used to create non-normalized Decimals. Not normalizing can be desired if you are sure the Decimal won't require normalization - for example, the Decimal.dConst constants that re set by library itself. However, this let's users create arbitary non-normalized Decimals:

Decimal.fromComponents_noNormalize(NaN, 10, 0)

This example creates Decimal with sign NaN - which is never supporsed to happen. Why is this so bad? Methods like toString need to check for such edge cases - check if sign is NaN, check if both layer and mag are 0 before you can assume the Decimal is truly 0. This all slows down the library. I can't think of good solution for this - currently there probably isn't one. Once private class fields get added it might be good idea to make methods that create non-normalized Decimals private to the class.

[Patashu]

Former I think is worth fixing. When it doesn't introduce huge performance losses, basic stuff should work the way it works in Number.

Latter is working as intended. Only use fc_nn if you know what you're doing or you're willing to accept arbitrary consequences.

[jakub791]

Ok, thanks for answer. Honestly yeah, about the 2nd one, you should face the consequences of creating arbitary non-normalized Decimals.

[jakub791]

Hmm. Just throught about it now - new Decimal(true) is 0 which is also probably unexpected?

[Patashu]

I have no idea what that should be. Probably undefined behaviour - why would you ever write that on purpose?

[jakub791]

Because Number(true) is 1 shouldn't new Decimal(true) be 1 too?

[Patashu]

I guess! (Does break_infinity also fail this?)

[jakub791]

Uh, I don't know. I can check

[James103]

I just tested in FE000000, which uses break_infinity.min.js and new Decimal(true) throws TypeError: t.indexOf is not a function with stacktrace s.fromString, new s.

Throws exception from here (-1 !== t.indexOf("e")):

        s.prototype.fromString = function(t) {
            if (-1 !== t.indexOf("e")) {
                var n = t.split("e");

[jakub791]

Oh, because it tries to do fromString if input is not undefined, Decimal or number.

[MathCookie17]

First statement evaluates to true now, as 1.4.1 normalizes infinities. However, second statement is still false, as I set negative infinity to be normalized to have layer Infinity rather than negative Infinity. Patashu, can you explain why Decimal.dNegInf has layer -Infinity? Shouldn't it have sign -1, mag Infinity, layer Infinity? I guess I should have tested 1.4.1 a little more - I'll have to make a 1.4.2 fixing this, but first I think we need to decide what negative infinity should be and why.

Fourth and fifth statements are now false, as they should be. Third is still false, but that's actually intended - NaN is not equal to itself, that's a rule of floating point. In fact, Decimal.eq(Decimal.NaN, Decimal.NaN) returns false.

EDIT: Okay, I've submitted a pull request that makes the normalization of infinities have {-1, -Infinity, -Infinity} for negative infinity, and this makes your second statement true. Once that pull request is merged, this issue can be closed.

EDIT 2: This issue is solved - well, unless we care about the Decimal(true) thing.