On Thu, 29 Jun 2023 23:24:06 GMT, Naoto Sato <na...@openjdk.org> wrote:
>> Justin Lu has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove unusued JUnit imports from ValidateISO4217.java
>
> test/jdk/java/util/Currency/ValidateISO4217.java line 82:
>
>> 80: // Codes derived from ISO4217 golden-data file
>> 81: private static final List<Arguments> ISO4217Codes = new
>> ArrayList<Arguments>();
>> 82: // Additional codes not from the ISO4217 golden-data file
>
> Are these from the golden file?
Unless I am misinterpreting the question, `additionalCodes` is built from the
`extraCodes` array, which contains data that is not found in tablea1.txt(golden
file)
> test/jdk/java/util/Currency/ValidateISO4217.java line 96:
>
>> 94: setUpISO4217Codes();
>> 95: setUpAdditionalCodes();
>> 96: finishTestCurrencies();
>
> The method name `finish` is kind of confusing. I'd explicitly describe what
> the method does, i.e, setup `other` currencies.
Renamed it to your suggestion
> test/jdk/java/util/Currency/ValidateISO4217.java line 192:
>
>> 190: /* Defined neither in ISO 4217 nor ISO 3166 list */
>> 191: {"XK", "EUR", "978", "2"}, // Kosovo
>> 192: };
>
> I'd prefer this array to be placed as static in the class, not within the
> method.
Fixed, as well as for `otherCodes`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14682#discussion_r1247539421
PR Review Comment: https://git.openjdk.org/jdk/pull/14682#discussion_r1247540723
PR Review Comment: https://git.openjdk.org/jdk/pull/14682#discussion_r1247540191
