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

fix: handling of double and integer #316

Merged
merged 1 commit into from
Mar 8, 2023

Conversation

apulbere
Copy link
Contributor

@apulbere apulbere commented Mar 7, 2023

This PR

Notes

This is a proposal

@beeme1mr
Copy link
Member

beeme1mr commented Mar 7, 2023

Hey @apulbere, thanks for the PR. Someone will review it shortly.

@codecov
Copy link

codecov bot commented Mar 7, 2023

Codecov Report

Merging #316 (f4841b1) into main (09824e7) will decrease coverage by 0.40%.
The diff coverage is 69.23%.

@@             Coverage Diff              @@
##               main     #316      +/-   ##
============================================
- Coverage     91.80%   91.41%   -0.40%     
+ Complexity      213      212       -1     
============================================
  Files            23       23              
  Lines           488      489       +1     
  Branches         31       31              
============================================
- Hits            448      447       -1     
  Misses           24       24              
- Partials         16       18       +2     
Flag Coverage Δ
unittests 91.41% <69.23%> (-0.40%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/main/java/dev/openfeature/sdk/Structure.java 87.50% <66.66%> (-2.25%) ⬇️
src/main/java/dev/openfeature/sdk/Value.java 91.35% <71.42%> (-1.24%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Value doubleValue = new Value(innerDoubleValue);
assertTrue(doubleValue.isNumber());
assertEquals(1, doubleValue.asInteger()); // should be rounded
assertEquals(.75, doubleValue.asDouble());
assertEquals(1, doubleValue.asInteger()); // the double value represented by this object converted to type int
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for linking the spec related to this rounding "toward zero". I was not aware of this nuance.

Copy link
Contributor

@Kavindu-Dodan Kavindu-Dodan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. Yes it makes sense to store Number type instead of double.

@beeme1mr
Copy link
Member

beeme1mr commented Mar 7, 2023

@justinabrahms any concerns?

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@toddbaert toddbaert merged commit 0a27a77 into open-feature:main Mar 8, 2023
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

Successfully merging this pull request may close these issues.

4 participants