On Mon, 28 Mar 2022 17:02:33 GMT, Naoto Sato <[email protected]> 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
