<i18n dev> [15] RFR: 8227313: Support monetary grouping separator in DecimalFormat/DecimalFormatSymbols

[naoto.sato at oracle.com]

Hi Joe,

Happy new year and thanks for your comments. Please see my replies below:

On 12/23/19 5:20 PM, Joe Wang wrote:
> Hi Naoto,
> 
> Is there a need for an APINote to note the relationship between the new 
> get/setMonetaryGroupingSeparator and get/setGroupingSeparator methods. 
> The new method did state it "May be different from {@code grouping 
> separator} in some locales", but that may be insufficient. For example, 
> does setting one method affects the other (seems it should since the 
> monetaryGroupingSeparator may be initialized by the groupingSeparator as 
> the impl shows)? If yes, how it's affected?

Setting the custom monetary grouping separator will not affect the 
existing normal grouping separator. I added the explanation in the 
method description.

> 
> If no, is there a compatibility concern? In the current impl, the new 
> get method is used when "isCurrencyFormat" is true while the new set 
> method doesn't affect the existing 'groupingSeparator'. For existing 
> applications that have a custom grouping separator set through 
> setGroupingSeparator, it seems to me the custom separator won't be used 
> moving onto the new JDK impl.

Good point. Modified the compatibility risk from minimum to low with the 
explanation.

> 
> 
> A minor comment about the definition statement in the following:
> 
>      Add the following new serializable field 
> in|java.text.DecimalFormatSymbols|class:
> 
> |/** * The grouping separator used when formatting currency values. * * 
> @serial * @since 15 */ private char monetaryGroupingSeparator;|
> 
> 
> and that for the new get method:
>          Gets the character used for thousands separator for currencies.
> 
> 
> While the meanings are clear, they were not as consistent as that 
> between the existing groupingSeparator and its get method, that is:
>          Character used for thousands separator.
> 
>     and then the get method states:
> 
>          Gets the character used for thousands separator.
> 
> 
> But this is minor, your call whether to change it or not.

Consistency is important. I replaced all occurrences of "thousands 
separator(s)" in DecimalFormat/DecimalFormatSymbols with "grouping 
separator(s)."

Here are the modified changeset:

http://cr.openjdk.java.net/~naoto/8227313/webrev.01/

as well as the modified CSR at the same URL.

Naoto

> 
> Best,  and have a great Christmas! :-)
> Joe
> 
> On 12/20/19 12:57 PM, naoto.sato at oracle.com wrote:
>> Hi,
>>
>> Please review the fix for the following issue:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8227313
>>
>> The proposed CSR and changeset are located at:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8235942
>> https://cr.openjdk.java.net/~naoto/8227313/webrev.00/
>>
>> The change introduces the new monetary grouping separator in 
>> DecimalFormatSymbols, and it is used if a DecimalFormat instance 
>> designates currency formatting where its pattern includes the currency 
>> symbol (U+00A5). The change makes use of the CLDR data which provides 
>> two distinct grouping separators (one for generic, the other for 
>> currency) in some locales. It also addresses the similar cases for the 
>> decimal separator.
>>
>> Naoto
>