On Tue, 7 Oct 2025 23:34:49 GMT, Justin Lu <[email protected]> wrote:

>> Please review this PR which corrects a rounding error for DecimalFormat on 
>> tie cases when the maximum fraction digits allowed is one less than the 
>> position of the first significant digit in the double rounded. For example, 
>> rounding a value such as 0.0005 to a maximum of 3 fractional digits. In 
>> order to get the correct `count` value, we must strip the trailing zeros 
>> prior to the check to ensure we have the correct value of `count` = 1 so 
>> that we can go down the _Rounding position is the last index_ case which 
>> properly checks if the String double was already rounded or is exact from 
>> its true decimal form.
>
> test/jdk/java/text/Format/DecimalFormat/RoundingToLSDTieTest.java line 1:
> 
>> 1: /*
> 
> I considered adding the tests in this file under _TieRoundingTest_, but did 
> not do so to make review easier since JUnit could be used for the test in 
> this PR. (_TieRoundingTest_ is old and does not use JUnit.) Will consider 
> updating  _TieRoundingTest_ to use JUnit and merging the tests in this PR 
> under it in a separate issue.

I think "LSD" is a bit cryptic. I'd spell-out or replace with other words if 
possible (and the PR/JBS titles too)

> test/jdk/java/text/Format/NumberFormat/NumberRegression.java line 1779:
> 
>> 1777:                 "0%", "0%", "1%", "1%", "1%",
>> 1778:                 "0", "2", "0.2", "0.6", "0.04",
>> 1779:                 "0.04", "0.001", "0.002",
> 
> This test is checking that `.0005` rounded to 3 digits is "0.000" under 
> HALF_EVEN.
> 
> This is wrong. See,
> `new BigDecimal(0.0005) ==> 
> 0.0005000000000000000104083408558608425664715468883514404296875`. 
> 
> Resolving the bug in this PR requires fixing this incorrect test case. From 
> the JDK-4241880 issue, it seems this case may have been put in to "document 
> the incorrect but actual behavior".

I guess this is the exact case this fix is trying to address, but the expected 
result wrongly assumes the behavior.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/27681#discussion_r2414578536
PR Review Comment: https://git.openjdk.org/jdk/pull/27681#discussion_r2414581725

Reply via email to