<i18n dev> RFR: 8341445: DecimalFormatSymbols setters should throw NPE [v4]

Naoto Sato naoto at openjdk.org
Fri Oct 11 23:15:39 UTC 2024


On Fri, 11 Oct 2024 22:23:42 GMT, Justin Lu <jlu at openjdk.org> wrote:

>> Please review this PR which improves the safety of equality checking for DecimalFormatSymbols. As certain setters did not throw NPE, this allowed for NPE in the equality method. This PR now updates the setters to throw NPE.
>> 
>> In addition to the NPEs, there is also a behavioral change that `setInternationalCurrencySymbol` no longer sets currency to null if the `currencyCode` is invalid. Instead, it simply does not update `currency`. This must be done, because we do not want to allow nullable instance variables post `initalizeCurrency`.
>
> Justin Lu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   reflect review

Looks good overall. The test can also check the behavioral change in `setInternationalCurrencySymbol()`.

src/java.base/share/classes/java/text/DecimalFormatSymbols.java line 767:

> 765:             NaN.equals(other.NaN) &&
> 766:             // Currency fields are lazy. Init via get call to ensure non-null
> 767:             getCurrencySymbol().equals(other.getCurrencySymbol()) &&

Probably the same comment refinement can be applied to the location in `hashCode()`.

test/jdk/java/text/Format/DecimalFormat/SettersShouldThrowNPETest.java line 55:

> 53:             .filter(m -> Modifier.isPublic(m.getModifiers()))
> 54:             .filter(m -> m.getName().startsWith("set"))
> 55:             .filter(m -> Stream.of(m.getParameterTypes()).noneMatch(Class::isPrimitive))

Can consolidate into a single filter()?

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

PR Review: https://git.openjdk.org/jdk/pull/21315#pullrequestreview-2363659166
PR Review Comment: https://git.openjdk.org/jdk/pull/21315#discussion_r1797484163
PR Review Comment: https://git.openjdk.org/jdk/pull/21315#discussion_r1797485380


More information about the i18n-dev mailing list