<i18n dev> Request for Review : JDK-8071929 -Locale.getISOCountries() has inconsistent behaviour for "AN", "BU" and "CS" country codes.

[Naoto Sato]

Additional nit comments:

Locale.java

- Extra blank lines at line 628, 639.

- The method name "getISOCountriesImpl" inside IsoCountryCode is 
somewhat strange. How about just "createCountryCodeSet()"? Also, its 
comment does not describe the method correctly. It does not seem a 
"mapping function".

Naoto

On 12/2/16 2:35 AM, Rachna Goel wrote:
> Hi Stephen, Masayoshi,
>
> Thanks for review and suggestions.
>
> Please have a look at updated patch on this issue.
>
> http://cr.openjdk.java.net/~rgoel/JDK_8071929/webrev.03/index.html
>
> Updations in new patch:
>
> 1. Updated all method names to have "ISO".
>
> 2. Changed implementation of mapping function to be inside enum so that
> default case can be avoided, as suggested by Masayoshi.
>
> As suggested from Naoto, adding extra descriptions and clarifications
> can be handled with a separate JBS issue.
>
> Thanks,
> Rachna
>
>
>
> On 01/12/16 10:29 PM, Naoto Sato wrote:
>> I think what we can improve here is to add extra descriptions to each
>> enum constant. You can refer to the definitions to each country code
>> sets at the ISO site [1]. That clarification can be done with a
>> separate JBS issue.
>>
>> Naoto
>>
>> [1]:
>> http://www.iso.org/iso/home/standards/country_codes/country_codes_glossary.htm
>>
>> On 11/30/16 7:33 AM, Rachna Goel wrote:
>>>  Hi Stephen,
>>>
>>> Please find my comments inlined.
>>>
>>> On 30/11/16 5:07 PM, Stephen Colebourne wrote:
>>>> The ISO-3166-3 code is of a complex form and not widely used as far as
>>>> I know. As far as I can tell, it is not legal to create a Locale
>>>> instance using the ISO-3166-3 code as the country.
>>> Yes, Agreed..API doc of Locale constructors mentions that, which type of
>>> country codes are valid input.
>>>> At the very least, the PART3 constant should describe the meaning and
>>>> structure of the codes.
>>> I think, API doc of getISOCountries() to be updated mentions that :
>>>
>>>
>>> * ISO3166-3 codes which designate country codes for those obsolete
>>> codes,
>>>      * can be retrieved from {@link
>>> #getISOCountries(Locale.IsoCountryCode type)} with
>>>       * {@code type}  {@link IsoCountryCode#PART3}.
>>>>
>>>> What might be useful would be a much more detailed result, where a
>>>> user could lookup an old code and find out when it expired, and what
>>>> replaced it.
>>> I think that again depends on applications requirements.
>>> For this new method may need to be added, may be handled under separate
>>> JBS entry.
>>>> Stephen
>>>>
>>>>
>>>> On 30 November 2016 at 09:31, Rachna Goel <rachna.goel at oracle.com>
>>>> wrote:
>>>>> Hi Stephen,
>>>>>
>>>>> Thanks a lot for the review.
>>>>>
>>>>> On 30/11/16 3:15 AM, Stephen Colebourne wrote:
>>>>>
>>>>> I'm concerned that this is not the friendliest of new APIs. There is
>>>>> little description of the meaning of the ISO-3166 parts - what is
>>>>> being added is directly exposing the underlying data rather than
>>>>> providing any kind of abstraction.
>>>>>
>>>>> Could you throw more light on this? Country code data is already
>>>>> encapsulated, and there is no direct reference to map accessing those
>>>>> data.
>>>>> If you have some suggestions on improving it, kindly share.
>>>>>
>>>>> There is also an inconsistency between "ISO" and "Iso" in the
>>>>> class/method
>>>>> names.
>>>>>
>>>>> There has been lot of discussion regarding "ISO" and "Iso". CCC has
>>>>> suggested use of "IsoCountryCode" for enum name which I proposed to be
>>>>> "ISOCountrycode" .
>>>>>
>>>>> I have tried to keep constants as "ISO", variables as "iso" as per
>>>>> with JDK
>>>>> naming conventions. But Locale class has methods names with  "ISO",
>>>>> So I
>>>>> think I will update all internal method names to have "ISO".
>>>>>
>>>>> Thanks,
>>>>> Rachna
>>>>>
>>>>>
>>>>> Stephen
>>>>>
>>>>>
>>>>> On 29 November 2016 at 09:07, Rachna Goel <rachna.goel at oracle.com>
>>>>> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Please review fix for JDK-8071929.
>>>>>
>>>>> Bug : https://bugs.openjdk.java.net/browse/JDK-8071929
>>>>>
>>>>> patch : http://cr.openjdk.java.net/~rgoel/JDK_8071929/webrev.02/
>>>>>
>>>>> Fix is to remove obsolete country code "AN" and provide support for
>>>>> retrieving of ISO3166-1 alpha-2,  ISO3166-1 alpha-3, ISO3166-3 country
>>>>> codes.
>>>>>
>>>>> Thanks,
>>>>> Rachna
>>>>>
>>>>>
>>>
>