Typed numbers

[moonglum]

I'm using Message Pack to communicate between Java and JavaScript – using your great library (thanks for that!). We ran into a problem when transferring numbers though. The setup is as follows:

A Java program sends a request to Node.js (using Message Pack), Node.js does some work on the data, sends it to another Java program (using Message Pack). We noticed that the numbers were scrambled.

The reason for that is simple: You are always using the smallest number format possible to encode this specific number. This however leads to the effect described above, rendering the library unusable for us.

My solution introduces TypedNumbers which are tiny wrappers around JS Numbers:

var encoder = msgpack()
  , TypedNumber = encoder.TypedNumber
  , typedDouble = new TypedNumber(12, 'double')

buf = encoder.encode(typedDouble) // buf will now be encoded as a double

If you encode a JavaScript Number without a wrapper, it will continue to work as it does now – choose the smallest number type possible for encoding.

For decoding however, the library needs to know if you like typed numbers or "normal" numbers. Therefore I introduced an option in this PR:

var encoder = msgpack({ typedNumbers: true })

If you decode a double with this option you will receive a TypedNumber of the type double instead of a Number.

This PR is only implementing this functionality for doubles, as I wanted to get your feedback first before implementing it for all types of numbers. What do you think?

[moonglum]

Impact:

• As you can see, the existing unit tests still pass as the change is 100% backwards compatible.
• When encoding, a single case statement is added which tests if the object to be encoded has the attribute isTypedNumber set to a truthy value.
• When decoding a number a function is called. When the option is not active, this function simply returns the number. If the option is active, it will wrap the number in a TypedNumber.

[mcollina]

This is great work. However I am not sure this is how msgpack should work. Your problem is a JS one, not a msgpack one. In Javascript, all numbers are doubles. So, In order to detect if a number is a double or not, I have to resort to using Math.floor in https://github.com/mcollina/msgpack5/blob/master/lib/encoder.js#L229-L231.

I think a much better way for doing that is using the new ES6 Number.isInteger https://github.com/parshap/js-is-integer, and it will be way simpler. Could you give it a spin?

[moonglum]

Thanks for your reply :smile:

isInteger works exactly as your implementation, from the README:

isInteger(4.0) // => true

So this would not change the behavior of the library in any way. This, however, is the big problem: As there is no difference in JavaScript between 4.0 and 4, there is no way to determine if it should be an Integer or a Float. There can't be an implementation of isInteger that returns a different result for 4 and 4.0 in JavaScript.

Imagine you have an API that expects a Float. In the current implementation there is no way to send a Float with the value 4.0. This might be fine if the API is implemented in a language where there is no difference between Floats and Integers, but most languages see a difference between them.

[moonglum]

I think the difference between the current approach of the library and the approach of this PR is the following:

• Current: We only have numbers (as it works in JS). We automatically determine which number type is the smallest one that could fit this number.
• PR: Every number type is its own distinct type (as it is described in the msgPack Spec). We let the user decide, which type they want to use.

So the question is "Do you want it to behave like JS or like MsgPack?". And I think both view points are absolutely understandable and useful in different situations. This is why I would suggest making it configurable.

[mcollina]

I am more keen on working like JS.

However, does this patch have a performance impact? Can you please run these https://github.com/mcollina/msgpack5/tree/master/benchmarks before/after and report your findings?

If it does not have a performance impact it's in.

[moonglum]

We are using ProtoBuff now. I think that the use case described probably fits best with a typed serializer format. Therefore I will close this PR :wink:

[mcollina]

BTW, I'm using https://github.com/mafintosh/protocol-buffers for protocol buffers