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

Why the SDK does not provide a way to fetch Long value? #501

Open
apulbere opened this issue Jul 10, 2023 · 5 comments
Open

Why the SDK does not provide a way to fetch Long value? #501

apulbere opened this issue Jul 10, 2023 · 5 comments
Labels
community Improving the community and contributor experience Needs Triage This issue needs to be investigated by a maintainer

Comments

@apulbere
Copy link
Contributor

apulbere commented Jul 10, 2023

I see we have getDoubleValue and getIntegerValue, but not for Long type.
I can open a PR to add that.

@Kavindu-Dodan
Copy link
Contributor

@apulbere thank you for raising your concern.

From your description, I assume you are referring to the feature flag evaluation contract [1]. I am not sure about the historical reason for ignoring the Long type. However, it could be a valuable addition for providers.

From the spec point of view, we only enforce number evaluation [2]. When it comes to implementation, this translates into language-specific representations.

Maybe we could aim for a generic representation of numbers via Java Number [3] type evaluation instead of supporting each sub-type of Number such as Long and Short 🤔

cc - @toddbaert @justinabrahms appreciate opnion here

[1] - https://github.com/open-feature/java-sdk/blob/main/src/main/java/dev/openfeature/sdk/FeatureProvider.java#L16-L24
[2] - https://github.com/open-feature/spec/blob/main/specification/sections/01-flag-evaluation.md#13-flag-evaluation
[3] - https://docs.oracle.com/javase/8/docs/api/java/lang/Number.html

@justinabrahms
Copy link
Member

I'd support something around Number. I don't think I knew about it at the time.

@Kavindu-Dodan Kavindu-Dodan added community Improving the community and contributor experience Needs Triage This issue needs to be investigated by a maintainer labels Jul 10, 2023
@toddbaert
Copy link
Member

I think Number makes sense.

If we add it though, what do we do with providers that don't specifically implement this? Or would we even expect them to? Might we instead just call resolveDoublevalue on the provider and use that value to create a Number?

I'm not sure...

@aepfli
Copy link
Member

aepfli commented Oct 16, 2024

I like this idea :) I also like Todd's suggestion to maintain backward compatibility.

IMHO: we should

  • add NumberEvaluation as a default method to the interface and redirect it per default to the DoubleEvaluation (this way, we won't break existing providers)
  • Now, my question would be if we deprecate the existing ones or want to keep them in place with a default implementation pointing to NumberEvaluation (I know a possible circular call if both are not implemented).

This ticket has been open for quite a while, and I think we should continue with this improvement. So, what do you think? especially about having default methods for all kinds of number sub-types or not.

@toddbaert
Copy link
Member

I like your plan. I'm really not sure about your question though. My gut says deprecate the other numerics, but I'm not sure. 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Improving the community and contributor experience Needs Triage This issue needs to be investigated by a maintainer
Projects
None yet
Development

No branches or pull requests

5 participants