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

The LdValue.Convert.Long converter returns LdValue.AsInt instead of LdValue.AsLong #32

Closed
irwinb opened this issue Sep 24, 2019 · 3 comments

Comments

@irwinb
Copy link

irwinb commented Sep 24, 2019

Is this a support request?
No.

Describe the bug
The LdValue.Convert.Long converter returns LdValue.AsInt instead of LdValue.AsLong.

To reproduce
Initialize an LdValue with a long list and get it back.

var val = new List<long> {long.MinValue, 10, 100, long.MaxValue};
var ldValue = LdValue.Of(val );

// This fails.
Assert.True(val, ldValue.AsLongList());

Expected behavior
I expect a list with the correct values to be returned.

SDK version
4.0.1

@irwinb
Copy link
Author

irwinb commented Sep 24, 2019

I'm realizing that the test I posted might not be valid. What is the maximum numeric value LaunchDarkly supports?

@eli-darkly
Copy link
Contributor

eli-darkly commented Sep 27, 2019

  1. Thanks for spotting that error. Indeed, it should be using AsLong there.

  2. That's a fair question, but the answer isn't well defined. All values in LaunchDarkly flag rules and user properties are encoded as JSON-- in communication between the service and the SDKs, and also in how the service stores them internally. The JSON standard does not define any range or precision for numbers; in practice, most languages (including Go, which is what the LD service runs on) will represent them using the largest available floating-point type (which in Go is float64). Floating-point numbers do not have a maximum or minimum value, just a maximum precision.

Since the maximum floating-point precision is somewhat different from one language to another, it's not recommended to rely on being able to encode as many digits as a Go float64 can handle; for very large numbers, it'd be best to encode them as strings (assuming you don't need to use numeric comparison operators for them in a flag rule). Therefore, while the long conversion is provided here for convenience in situations where that is the type you have, its existence is somewhat misleading and it should probably have a warning that long values are not guaranteed to be encodable in JSON.

  1. I'm not entirely sure what that test code is supposed to be doing. It doesn't compile, since there is no overload of LdValue.Of() that takes List<long>, nor is there an AsLongList() method of LdValue.

@eli-darkly
Copy link
Contributor

The fix for this is now in .NET SDK 5.9.0 and Xamarin SDK 1.1.0.

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