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 checking a surrogate pair #232

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix checking a surrogate pair #232

wants to merge 3 commits into from

Conversation

t-mangoe
Copy link

@t-mangoe t-mangoe commented Apr 7, 2019

I fixed code. I think this fix solves the issue #207 . If you like, please merge.

@codecov-io
Copy link

codecov-io commented Apr 7, 2019

Codecov Report

Merging #232 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #232      +/-   ##
==========================================
- Coverage   68.46%   68.45%   -0.01%     
==========================================
  Files         107      107              
  Lines        7335     7336       +1     
  Branches     1388     1388              
==========================================
  Hits         5022     5022              
- Misses       1867     1868       +1     
  Partials      446      446
Impacted Files Coverage Δ
...ain/java/com/jsoniter/output/StreamImplString.java 53.91% <0%> (-0.48%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e48a7a1...24f9551. Read the comment docs.

BigDecimal integerPart = new BigDecimal(value);
BigDecimal fractionalPart = new BigDecimal((decimalPart / (double) IterImplNumber.POW10[decimalPlaces]));
BigDecimal result = integerPart.add(fractionalPart).setScale(decimalPlaces,BigDecimal.ROUND_HALF_UP);
return result.doubleValue();
Copy link
Contributor

Choose a reason for hiding this comment

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

@t-mangoe do you have tests and benchmarks that prove need for this change?

also I would move this commit to the separated pull request

Copy link
Author

@t-mangoe t-mangoe Apr 13, 2019

Choose a reason for hiding this comment

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

@plokhotnyuk Sorry. This commit targets another issue. I will revert this commit, and make separeted pull request. Thanks.

@plokhotnyuk
Copy link
Contributor

plokhotnyuk commented Apr 13, 2019

@t-mangoe please add tests for that case

best option would be just checking of a whole range of surrogate pairs in some loop with an assertion for each pair

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.

None yet

3 participants