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.g...@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.g...@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
