Re: RFR: 8282819: Deprecate Locale class constructors [v7]
On Tue, 29 Mar 2022 21:28:44 GMT, Naoto Sato wrote: >> Proposing to deprecate the constructors in the `java.util.Locale` class. >> There is already a factory method and a builder to return singletons, so >> there is no need to have constructors anymore unless one purposefully wants >> to create `ill-formed` Locale objects, which is discouraged. We cannot >> terminally deprecate those constructors for the compatibility to serialized >> objects created with older JDKs. Please see the draft CSR for more detail. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Changed from var-args to conventional args. Reflected suggestions. Per a comment in the CSR discussion (https://bugs.openjdk.java.net/browse/JDK-8283478?focusedCommentId=14485037=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14485037), I changed the arguments to var-args to conventional ones. - PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v7]
> Proposing to deprecate the constructors in the `java.util.Locale` class. > There is already a factory method and a builder to return singletons, so > there is no need to have constructors anymore unless one purposefully wants > to create `ill-formed` Locale objects, which is discouraged. We cannot > terminally deprecate those constructors for the compatibility to serialized > objects created with older JDKs. Please see the draft CSR for more detail. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Changed from var-args to conventional args. Reflected suggestions. - Changes: - all: https://git.openjdk.java.net/jdk/pull/7947/files - new: https://git.openjdk.java.net/jdk/pull/7947/files/b1d36b52..8ac9d11d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7947=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7947=05-06 Stats: 132 lines in 4 files changed: 76 ins; 3 del; 53 mod Patch: https://git.openjdk.java.net/jdk/pull/7947.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7947/head:pull/7947 PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v6]
On Mon, 28 Mar 2022 22:16:43 GMT, Naoto Sato wrote:
>> Proposing to deprecate the constructors in the `java.util.Locale` class.
>> There is already a factory method and a builder to return singletons, so
>> there is no need to have constructors anymore unless one purposefully wants
>> to create `ill-formed` Locale objects, which is discouraged. We cannot
>> terminally deprecate those constructors for the compatibility to serialized
>> objects created with older JDKs. Please see the draft CSR for more detail.
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Added an @apiNote describing the single array argument
src/java.base/share/classes/sun/util/locale/provider/JRELocaleConstants.java
line 38:
> 36: */
> 37: public class JRELocaleConstants {
> 38: public static final Locale JA_JP_JP =
> Locale.forLanguageTag("ja-JP-x-lvariant-JP");
You might use the new factory here.
src/java.base/share/classes/sun/util/resources/LocaleData.java line 249:
> 247: // TODO: avoid hard-coded Locales
> 248: private static final Set JAVA_BASE_LOCALES
> 249: = Set.of(Locale.ROOT, Locale.ENGLISH, Locale.US,
> Locale.forLanguageTag("en-US-POSIX"));
Use new factory?
-
PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v3]
On Mon, 28 Mar 2022 21:06:35 GMT, Roger Riggs wrote: >> While it is true for completeness, I would limit the addition of new method >> as little as possible, because there are already several ways to obtain a >> Locale object. > > As with other varargs method calls, it is possible to pass an array with the > values. > I think it would be useful to describe the arguments as using the varargs > nomenclature and indicate > the values are in the array. For example, the `java.util.List.of(E... > elements)` method is explicit > about the array. Anther API using varargs EnumSet. Added the explanation following the `List.of(E... elements)` javadoc. - PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v6]
> Proposing to deprecate the constructors in the `java.util.Locale` class. > There is already a factory method and a builder to return singletons, so > there is no need to have constructors anymore unless one purposefully wants > to create `ill-formed` Locale objects, which is discouraged. We cannot > terminally deprecate those constructors for the compatibility to serialized > objects created with older JDKs. Please see the draft CSR for more detail. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Added an @apiNote describing the single array argument - Changes: - all: https://git.openjdk.java.net/jdk/pull/7947/files - new: https://git.openjdk.java.net/jdk/pull/7947/files/86c7b586..b1d36b52 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7947=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7947=04-05 Stats: 4 lines in 2 files changed: 4 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7947.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7947/head:pull/7947 PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v3]
On Mon, 28 Mar 2022 16:00:14 GMT, Naoto Sato wrote:
>> src/java.base/share/classes/java/util/Locale.java line 819:
>>
>>> 817: * @since 19
>>> 818: */
>>> 819: public static Locale of(String... fields) {
>>
>> Arguably, there should be `Locale.of` overloads taking 0 to 4 arguments, so
>> that it’s not necessary to box the fields in a `String` array.
>
> While it is true for completeness, I would limit the addition of new method
> as little as possible, because there are already several ways to obtain a
> Locale object.
As with other varargs method calls, it is possible to pass an array with the
values.
I think it would be useful to describe the arguments as using the varargs
nomenclature and indicate
the values are in the array. For example, the `java.util.List.of(E...
elements)` method is explicit
about the array. Anther API using varargs EnumSet.
-
PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v5]
On Mon, 28 Mar 2022 18:51:30 GMT, Naoto Sato wrote: >> Proposing to deprecate the constructors in the `java.util.Locale` class. >> There is already a factory method and a builder to return singletons, so >> there is no need to have constructors anymore unless one purposefully wants >> to create `ill-formed` Locale objects, which is discouraged. We cannot >> terminally deprecate those constructors for the compatibility to serialized >> objects created with older JDKs. Please see the draft CSR for more detail. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Refined test cases Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v4]
On Mon, 28 Mar 2022 17:13:44 GMT, Lance Andersen wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> New unit test. IllegalArgumentException.
>
> test/jdk/java/util/Locale/TestOf.java line 79:
>
>> 77: @Test (expectedExceptions = IllegalArgumentException.class)
>> 78: public void test_IAE() {
>> 79: Locale.of("en", "", "", "", "");
>
> I would consider using `assertThrows(IllegalArgumentException.class, () ->
> Locale.of("en", "", "", "", "")); ` instead of the expectedExceptions
> annotation element as it is the preferred way forward
Thanks. Modified as suggested.
-
PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v5]
> Proposing to deprecate the constructors in the `java.util.Locale` class. > There is already a factory method and a builder to return singletons, so > there is no need to have constructors anymore unless one purposefully wants > to create `ill-formed` Locale objects, which is discouraged. We cannot > terminally deprecate those constructors for the compatibility to serialized > objects created with older JDKs. Please see the draft CSR for more detail. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Refined test cases - Changes: - all: https://git.openjdk.java.net/jdk/pull/7947/files - new: https://git.openjdk.java.net/jdk/pull/7947/files/9d9d3a69..86c7b586 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7947=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7947=03-04 Stats: 9 lines in 1 file changed: 4 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/7947.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7947/head:pull/7947 PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v4]
On Mon, 28 Mar 2022 17:02:33 GMT, Naoto Sato wrote:
>> Proposing to deprecate the constructors in the `java.util.Locale` class.
>> There is already a factory method and a builder to return singletons, so
>> there is no need to have constructors anymore unless one purposefully wants
>> to create `ill-formed` Locale objects, which is discouraged. We cannot
>> terminally deprecate those constructors for the compatibility to serialized
>> objects created with older JDKs. Please see the draft CSR for more detail.
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> New unit test. IllegalArgumentException.
Changes look fine.
I would suggest adding a comment describing the new tests. Also one additional
suggestion below
test/jdk/java/util/Locale/TestOf.java line 79:
> 77: @Test (expectedExceptions = IllegalArgumentException.class)
> 78: public void test_IAE() {
> 79: Locale.of("en", "", "", "", "");
I would consider using `assertThrows(IllegalArgumentException.class, () ->
Locale.of("en", "", "", "", "")); ` instead of the expectedExceptions
annotation element as it is the preferred way forward
-
Marked as reviewed by lancea (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v4]
> Proposing to deprecate the constructors in the `java.util.Locale` class. > There is already a factory method and a builder to return singletons, so > there is no need to have constructors anymore unless one purposefully wants > to create `ill-formed` Locale objects, which is discouraged. We cannot > terminally deprecate those constructors for the compatibility to serialized > objects created with older JDKs. Please see the draft CSR for more detail. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: New unit test. IllegalArgumentException. - Changes: - all: https://git.openjdk.java.net/jdk/pull/7947/files - new: https://git.openjdk.java.net/jdk/pull/7947/files/b4d040af..9d9d3a69 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7947=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7947=02-03 Stats: 87 lines in 2 files changed: 83 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/7947.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7947/head:pull/7947 PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v3]
On Sun, 27 Mar 2022 08:45:01 GMT, ExE Boss wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fixed a build failure
>
> src/java.base/share/classes/java/util/Locale.java line 819:
>
>> 817: * @since 19
>> 818: */
>> 819: public static Locale of(String... fields) {
>
> Arguably, there should be `Locale.of` overloads taking 0 to 4 arguments, so
> that it’s not necessary to box the fields in a `String` array.
While it is true for completeness, I would limit the addition of new method as
little as possible, because there are already several ways to obtain a Locale
object.
> src/java.base/share/classes/java/util/Locale.java line 825:
>
>> 823: case 2 -> getInstance(fields[0], "", fields[1], "", null);
>> 824: case 3 -> getInstance(fields[0], "", fields[1], fields[2],
>> null);
>> 825: default -> getInstance(fields[0], fields[3], fields[1],
>> fields[2], null);
>
> This should probably throw `IllegalArgumentException` when more than 4 fields
> are passed:
> Suggestion:
>
> case 4 -> getInstance(fields[0], fields[3], fields[1], fields[2],
> null);
> default -> throw new IllegalArgumentException(/* TODO: message
> */);
Thanks for the suggestion. Will incorporate the exception in the spec/impl.
-
PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v3]
On Sat, 26 Mar 2022 11:30:53 GMT, Lance Andersen wrote: > One suggestion As part of the PR, we should probably update > test/jdk/java/util/Locale/LocaleTest.java. or perhaps add a new test for > Locale.of() for completeness. Thanks, Lance. Sure, I will add some new unit tests for the new method. - PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v3]
On Fri, 25 Mar 2022 22:51:23 GMT, Naoto Sato wrote:
>> Proposing to deprecate the constructors in the `java.util.Locale` class.
>> There is already a factory method and a builder to return singletons, so
>> there is no need to have constructors anymore unless one purposefully wants
>> to create `ill-formed` Locale objects, which is discouraged. We cannot
>> terminally deprecate those constructors for the compatibility to serialized
>> objects created with older JDKs. Please see the draft CSR for more detail.
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fixed a build failure
src/java.base/share/classes/java/util/Locale.java line 819:
> 817: * @since 19
> 818: */
> 819: public static Locale of(String... fields) {
Arguably, there should be `Locale.of` overloads taking 0 to 4 arguments, so
that it’s not necessary to box the fields in a `String` array.
src/java.base/share/classes/java/util/Locale.java line 825:
> 823: case 2 -> getInstance(fields[0], "", fields[1], "", null);
> 824: case 3 -> getInstance(fields[0], "", fields[1], fields[2],
> null);
> 825: default -> getInstance(fields[0], fields[3], fields[1],
> fields[2], null);
This should probably throw `IllegalArgumentException` when more than 4 fields
are passed:
Suggestion:
case 4 -> getInstance(fields[0], fields[3], fields[1], fields[2],
null);
default -> throw new IllegalArgumentException(/* TODO: message */);
-
PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v3]
On Fri, 25 Mar 2022 22:51:23 GMT, Naoto Sato wrote: >> Proposing to deprecate the constructors in the `java.util.Locale` class. >> There is already a factory method and a builder to return singletons, so >> there is no need to have constructors anymore unless one purposefully wants >> to create `ill-formed` Locale objects, which is discouraged. We cannot >> terminally deprecate those constructors for the compatibility to serialized >> objects created with older JDKs. Please see the draft CSR for more detail. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed a build failure Hi Naoto, I think the changes look good. One suggestion As part of the PR, we should probably update test/jdk/java/util/Locale/LocaleTest.java. or perhaps add a new test for Locale.of() for completeness. - PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v3]
> Proposing to deprecate the constructors in the `java.util.Locale` class. > There is already a factory method and a builder to return singletons, so > there is no need to have constructors anymore unless one purposefully wants > to create `ill-formed` Locale objects, which is discouraged. We cannot > terminally deprecate those constructors for the compatibility to serialized > objects created with older JDKs. Please see the draft CSR for more detail. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Fixed a build failure - Changes: - all: https://git.openjdk.java.net/jdk/pull/7947/files - new: https://git.openjdk.java.net/jdk/pull/7947/files/e874ff7e..b4d040af Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7947=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7947=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7947.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7947/head:pull/7947 PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v2]
On Fri, 25 Mar 2022 01:56:33 GMT, Naoto Sato wrote: >> src/java.base/share/classes/sun/util/locale/provider/LocaleServiceProviderPool.java >> line 375: >> >>> 373: (locale.getLanguage().isEmpty() ? "und" : >>> locale.getLanguage()) + >>> 374: (locale.getCountry().isEmpty() ? "" : "-" + >>> locale.getCountry()) + >>> 375: (locale.getVariant().isEmpty() ? "" : >>> "-x-lvariant-" + locale.getVariant())); >> >> It seems like this snippet (and ones very similar to it) are repeated >> several times throughout the JDK code as replacements for the two- and >> three-arg constructors. This seems like a fair increase in complexity, and >> the use of "und" and "-x-lvariant-" are quite non-obvious. Would we >> recommend that third party code that uses the Locale constructors replace >> them with this snippet? Is there something better that we can provide? > > True. One solution could be to expose `Locale.getInstance()`, which is > currently a package-private static method, for this purpose. Though that > means promoting `ill-formed` locale objects which defy some part of the > deprecation cause. Introduced a new `Locale.of(String...)` method. - PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v2]
> Proposing to deprecate the constructors in the `java.util.Locale` class. > There is already a factory method and a builder to return singletons, so > there is no need to have constructors anymore unless one purposefully wants > to create `ill-formed` Locale objects, which is discouraged. We cannot > terminally deprecate those constructors for the compatibility to serialized > objects created with older JDKs. Please see the draft CSR for more detail. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Added a new factory method, removed pure refactoring from Locale API changes. - Changes: - all: https://git.openjdk.java.net/jdk/pull/7947/files - new: https://git.openjdk.java.net/jdk/pull/7947/files/9ff69cbe..e874ff7e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7947=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7947=00-01 Stats: 887 lines in 193 files changed: 37 ins; 67 del; 783 mod Patch: https://git.openjdk.java.net/jdk/pull/7947.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7947/head:pull/7947 PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors
On Fri, 25 Mar 2022 15:37:36 GMT, Roger Riggs wrote:
>> Proposing to deprecate the constructors in the `java.util.Locale` class.
>> There is already a factory method and a builder to return singletons, so
>> there is no need to have constructors anymore unless one purposefully wants
>> to create `ill-formed` Locale objects, which is discouraged. We cannot
>> terminally deprecate those constructors for the compatibility to serialized
>> objects created with older JDKs. Please see the draft CSR for more detail.
>
> src/java.base/share/classes/java/util/Locale.java line 245:
>
>> 243: * Factory Method
>> 244: *
>> 245: * The method {@link #forLanguageTag} obtains a {@code Locale}
>
> The factory name `forLanguageTag` is a bit off-putting, it doesn't seem like
> the best name for the factory.
> Yes, it already exists and does what's required but you might get better
> uptake with a more natural name.
>
> Some alternatives:
> - `Locale.of("en_US")` - short and conventional
> - `Locale.ofLanguage("en_US")` - 'of' prefix is used in other factories
> - `Locale.forLanguage("en_US")` - natural but less conventional
I was thinking of a *new* factory method, along the line with Stuart's
suggestion, something like this:
Locale.of(String... elements)
where elements can either `(lang)`, `(lang, ctry)`, `(lang, ctry, vrnt)`, or
`(lang, ctry, vrnt, scpt)`. Either element can be an empty string, but cannot
be null. These elements are *not* language tags, but conventional arguments to
constructors, so it is compatible (and works as a stop-gap) to the old
constructors. This way third parties will not have to deal with the boilerplate
code mentioned above on migration.
-
PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors
On Thu, 24 Mar 2022 22:01:30 GMT, Naoto Sato wrote: > Proposing to deprecate the constructors in the `java.util.Locale` class. > There is already a factory method and a builder to return singletons, so > there is no need to have constructors anymore unless one purposefully wants > to create `ill-formed` Locale objects, which is discouraged. We cannot > terminally deprecate those constructors for the compatibility to serialized > objects created with older JDKs. Please see the draft CSR for more detail. Given the large number of cleanups, I would have suggested to keep them separate from the change to re-focus the API on factories. - PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors
On Thu, 24 Mar 2022 22:01:30 GMT, Naoto Sato wrote:
> Proposing to deprecate the constructors in the `java.util.Locale` class.
> There is already a factory method and a builder to return singletons, so
> there is no need to have constructors anymore unless one purposefully wants
> to create `ill-formed` Locale objects, which is discouraged. We cannot
> terminally deprecate those constructors for the compatibility to serialized
> objects created with older JDKs. Please see the draft CSR for more detail.
src/java.base/share/classes/java/util/Locale.java line 245:
> 243: * Factory Method
> 244: *
> 245: * The method {@link #forLanguageTag} obtains a {@code Locale}
The factory name `forLanguageTag` is a bit off-putting, it doesn't seem like
the best name for the factory.
Yes, it already exists and does what's required but you might get better uptake
with a more natural name.
Some alternatives:
- `Locale.of("en_US")` - short and conventional
- `Locale.ofLanguage("en_US")` - 'of' prefix is used in other factories
- `Locale.forLanguage("en_US")` - natural but less conventional
-
PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors
On Fri, 25 Mar 2022 00:18:54 GMT, Stuart Marks wrote: >> Proposing to deprecate the constructors in the `java.util.Locale` class. >> There is already a factory method and a builder to return singletons, so >> there is no need to have constructors anymore unless one purposefully wants >> to create `ill-formed` Locale objects, which is discouraged. We cannot >> terminally deprecate those constructors for the compatibility to serialized >> objects created with older JDKs. Please see the draft CSR for more detail. > > src/java.base/share/classes/sun/util/locale/provider/LocaleServiceProviderPool.java > line 375: > >> 373: (locale.getLanguage().isEmpty() ? "und" : >> locale.getLanguage()) + >> 374: (locale.getCountry().isEmpty() ? "" : "-" + >> locale.getCountry()) + >> 375: (locale.getVariant().isEmpty() ? "" : >> "-x-lvariant-" + locale.getVariant())); > > It seems like this snippet (and ones very similar to it) are repeated several > times throughout the JDK code as replacements for the two- and three-arg > constructors. This seems like a fair increase in complexity, and the use of > "und" and "-x-lvariant-" are quite non-obvious. Would we recommend that third > party code that uses the Locale constructors replace them with this snippet? > Is there something better that we can provide? True. One solution could be to expose `Locale.getInstance()`, which is currently a package-private static method, for this purpose. Though that means promoting `ill-formed` locale objects which defy some part of the deprecation cause. - PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors
On Thu, 24 Mar 2022 22:01:30 GMT, Naoto Sato wrote:
> Proposing to deprecate the constructors in the `java.util.Locale` class.
> There is already a factory method and a builder to return singletons, so
> there is no need to have constructors anymore unless one purposefully wants
> to create `ill-formed` Locale objects, which is discouraged. We cannot
> terminally deprecate those constructors for the compatibility to serialized
> objects created with older JDKs. Please see the draft CSR for more detail.
Specs looks good, with minor modifications. I'm not so sure about replacement
of the constructors with string concatenation with ternary operators; see my
other comment.
src/java.base/share/classes/java/util/Locale.java line 252:
> 250: * The {@code Locale} class provides a number of convenient constants
> 251: * that you can use to obtain {@code Locale} objects for commonly used
> 252: * locales. For example, the following obtains a {@code Locale} object
I'd replace this part (and the example below) with something like,
For example, {@code Locale.US} is the {@code Locale} object for the United
States.
src/java.base/share/classes/java/util/Locale.java line 2511:
> 2509: * constructors, the {@code Builder} checks if a value configured
> by a
> 2510: * setter satisfies the syntax requirements defined by the {@code
> Locale}
> 2511: * class. A {@code Locale} object obtained by a {@code Builder} is
...obtained from a Builder...
src/java.base/share/classes/java/util/Locale.java line 2526:
> 2524: *
> 2525: * The following example shows how to obtain a {@code Locale}
> object
> 2526: * with the {@code Builder}.
...shows how to obtain a Locale object using a Builder.
src/java.base/share/classes/java/util/Locale.java line 2660:
> 2658: *
> 2659: * The country value in the {@code Locale} obtained by the
> 2660: * {@code Builder} is always normalized to upper case.
...obtained from a Builder...
src/java.base/share/classes/sun/util/locale/provider/LocaleServiceProviderPool.java
line 375:
> 373: (locale.getLanguage().isEmpty() ? "und" :
> locale.getLanguage()) +
> 374: (locale.getCountry().isEmpty() ? "" : "-" +
> locale.getCountry()) +
> 375: (locale.getVariant().isEmpty() ? "" :
> "-x-lvariant-" + locale.getVariant()));
It seems like this snippet (and ones very similar to it) are repeated several
times throughout the JDK code as replacements for the two- and three-arg
constructors. This seems like a fair increase in complexity, and the use of
"und" and "-x-lvariant-" are quite non-obvious. Would we recommend that third
party code that uses the Locale constructors replace them with this snippet? Is
there something better that we can provide?
-
PR: https://git.openjdk.java.net/jdk/pull/7947
