On Wed, 6 Dec 2023 15:27:48 GMT, Roger Riggs <rri...@openjdk.org> wrote:
>> Currently, Locale-related system properties, such as `user.language` or
>> `user.country`, are initialized when the `Locale` class is loaded. Making
>> them static properties is safer than relying on the `Locale` class loading
>> timing, which could potentially be changed depending on the implementation.
>
> src/java.base/share/classes/jdk/internal/util/StaticProperty.java line 113:
>
>> 111: USER_EXTENSIONS_DISPLAY = getProperty(props,
>> "user.extensions.display", USER_EXTENSIONS);
>> 112: USER_EXTENSIONS_FORMAT = getProperty(props,
>> "user.extensions.format", USER_EXTENSIONS);
>> 113: USER_REGION = getProperty(props, "user.region", "");
>
> Computing the defaults for these properties should be in the Locale class,
> close to the initialization of the default locale. Splitting the
> responsibility across files makes it harder to follow what happens where/when.
If I am not mistaken, these assignments should stay here in `StaticProperty`,
as that is the whole purpose of this change. Depending on `Locale` class
loading time might be fragile, and this change guarantees the properties are
initialized and frozen in `System.initPhase1()`
> src/java.base/share/classes/jdk/internal/util/StaticProperty.java line 415:
>
>> 413: public static String userRegion() {
>> 414: return USER_REGION;
>> 415: }
>
> Using methods to retrieve these makes is more complicated.
> The bleeding of the enum values outside of Locale is undesirable.
> Since the property values are final strings, I suggest just making the fields
> public and keep the mapping local to the Locale class.
As Alan commented, I will not use `Locale.Category.ordinal()` but instead use
the properties keys. Would that address your suggestion?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16986#discussion_r1417793505
PR Review Comment: https://git.openjdk.org/jdk/pull/16986#discussion_r1417785427
