<i18n dev> [14] RFR: 8230284: Accounting currency format support does not cope with explicit number system.

[Roger Riggs]

Hi,

Looks fine.

[The code is clear as is; but the shortcuts in Bundle.java (770-775) 
won't actually be a shortcut
because for each method it has to compute the value of the argument 
before it can call the method.
There isn't an API that accepts a supplier that would not be evaluated 
until the value was needed.]

Thanks, Roger


On 9/9/19 2:23 PM, naoto.sato at oracle.com wrote:
> Hi Roger,
>
> Thanks for the review.
>
> On 9/9/19 8:12 AM, Roger Riggs wrote:
>> Hi Naoto,
>>
>> Bundle.java:
>>   - 28: Explicit import are preferred,  (if the IDE make the change, 
>> fix the settings in the IDE).
>
> Corrected.
>
>>
>>   - 763, the indentation of the nested getOrDefault calls doesn't 
>> look conventional.
>>    (It might be interesting to have an API that allows one or more 
>> defaults to be tried in turn.
>>     It will call each of the methods so there is no shortcutting if 
>> the value is found early in the list).
>
> The code meant to short cut if a value is found early, as this 
> implements the fallback. I corrected the indentation of getOrDefault() 
> methods.
>



>>
>> CLDRConverter:
>>   - 513, 1169...: Debugging code can be removed
>
> I removed the one around line 513, but kept the 1169 as this is 
> useful. It does not affect the JDK image as this is the code for the 
> build tool.
>
>>
>> Otherwise looks ok.
>
> Updated webrev is here:
>
> http://cr.openjdk.java.net/~naoto/8230284/webrev.01/
>
> Naoto
>
>>
>> Thanks, Roger
>>
>>
>>
>> On 9/6/19 1:59 PM, naoto.sato at oracle.com wrote:
>>> Hello,
>>>
>>> Please review the fix to the following issue:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8230284
>>>
>>> The proposed changeset is located at:
>>>
>>> https://cr.openjdk.java.net/~naoto/8230284/webrev.00/
>>>
>>> The original enhancement for the accounting currency format support 
>>> (https://bugs.openjdk.java.net/browse/JDK-8215181) did not account 
>>> for the explicit/implicit numbering script. The above change made it 
>>> to share the same code with NumberElements which properly deals with 
>>> the numbering script.
>>>
>>> Naoto
>>