On Wed, 3 Nov 2021 11:53:52 GMT, Stephen Colebourne <scolebou...@openjdk.org>
wrote:
>> Claes Redestad has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add fallback for values outside the allowable range
>
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java
> line 3158:
>
>> 3156:
>> 3157: // only instantiated and used if there's ever a value outside
>> the allowed range
>> 3158: private FractionPrinterParser fallback;
>
> This class has to be safe in a multi-threaded environment. I'm not convinced
> it is safe right now, as the usage doesn't follow the standard racy single
> check idiom. At a minimum, there needs to be a comment explaining the
> thread-safety issues.
Yes, I'll make sure to read into a local variable.
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java
> line 3266:
>
>> 3264: if (!field.range().isValidIntValue(value)) {
>> 3265: if (fallback == null) {
>> 3266: fallback = new FractionPrinterParser(field,
>> minWidth, maxWidth, decimalPoint, subsequentWidth);
>
> It would be nice to see a test case cover this.
I'll see to it.
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java
> line 3290:
>
>> 3288: range.checkValidValue(value, field);
>> 3289: BigDecimal minBD = BigDecimal.valueOf(range.getMinimum());
>> 3290: BigDecimal rangeBD =
>> BigDecimal.valueOf(range.getMaximum()).subtract(minBD).add(BigDecimal.ONE);
>
> I wouldn't be surprised if there is a way to replace the use of `BigDecimal`
> with calculations using `long`. Fundamentally, calculations like 15/60 ->
> 0.25 are not hard, but it depends on whether the exact results can be matched
> across a wide range of possible inputs.
I think the main engineering challenge is writing tests that ensure that we
don't lose precision on an arbitrary fractional field.
-------------
PR: https://git.openjdk.java.net/jdk/pull/6188
