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

Serialize numeric types with smallest lossless format #2

Open
timothy-shields opened this issue Jun 24, 2015 · 2 comments
Open

Serialize numeric types with smallest lossless format #2

timothy-shields opened this issue Jun 24, 2015 · 2 comments

Comments

@timothy-shields
Copy link

What do you think about serializing numeric types with the smallest lossless format? For example, if I have a double[] of length N containing 0's and 1's, its contents (after the header) would be stored using N bytes, because 0 and 1 fit losslessly into a positive fixnum. If I try to then deserialize to a double[], that would work.

msgpack-cli is already doing this for ints and uints internally.

The implementation of DoubleMsgPackValue.Pack would become something logically similar to this:

public override void Pack(Packer packer)
{
    if (this.Value == (int)this.Value)
    {
        packer.Pack(this.Value < 0 ? (int)this.Value : (uint)this.Value);
    }
    else if (this.Value == (float)this.Value)
    {
        packer.Pack((float)this.Value);
    }
    else
    {
        packer.Pack(this.Value);
    }
}

What are your thoughts? This could potentially be an optional configuration on the MessagePackWriter. I'd be willing to help out with this.

@timothy-shields
Copy link
Author

I've actually tried this locally now, and it's working great. Deserialization appears to already handle this without any changes. That is, if you try to deserialize a uint fixnum containing the value 0 (hex 00) into a C# double that works correctly.

@darkl
Copy link
Member

darkl commented Jun 24, 2015

Hi,

The feature sounds nice,
You are welcome to propose a pull request with your changes. I think it is preferred this feature can be configurable.

I encourage you to propose further changes/improvements to this library. This project could use a little love.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants