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

[Rachna Goel]

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
>>>>
>>>>
>>