[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-i18n-dev
Subject:    Re: <i18n dev> [14] RFR: 8230284: Accounting currency format support does not cope with explicit num
From:       naoto.sato () oracle ! com
Date:       2019-09-09 19:21:57
Message-ID: c31dab55-7b19-5e14-4073-a0d221682aaa () oracle ! com
[Download RAW message or body]

Thanks again, Roger.

I now get what you meant. Yes, the current code does calculate each 
value before getting the correct one.

Naoto

On 9/9/19 12:07 PM, Roger Riggs wrote:
> 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@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@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
>>>
> 
[prev in list] [next in list] [prev in thread] [next in thread]