On Fri, 2 Feb 2024 22:11:17 GMT, Justin Lu <j...@openjdk.org> wrote:
>> src/java.base/share/classes/java/text/MessageFormat.java line 676:
>>
>>> 674: * java.time.format.DateTimeFormatter}. In addition, {@code
>>> DateTimeFormatter}
>>> 675: * does not implement {@code equals()}, and thus cannot be
>>> represented as a
>>> 676: * pattern. Any "default"/"medium" styles are omitted according to
>>> the specification.
>>
>> Since `DateTimeFormatter` eventually are converted to j.t.Format with
>> `toFormat()` method, wouldn't it be possible to implement this method for
>> those dtf(s)?
>
> We discussed this separately, but will go over again here in case others are
> interested. Since DTF does not implement `equals()`, even if we implement
> `equals()` for the `ClassicFormat`, we would basically still need to
> implement our own `equals()` for a DTF to effectively compare the
> ClassicFormats.
>
> I had also mentioned that we could reference check the pre-defined DTFs, but
> this won't work either actually. This is because we cannot reference check
> the DTFs, but rather the Format adapted DTFs, which mean they are now new
> ClassicFormats every time since we don't store them.
Thanks. I think it is fine.
>> src/java.base/share/classes/java/text/MessageFormat.java line 1904:
>>
>>> 1902: throw new IllegalArgumentException();
>>> 1903: }
>>> 1904: }
>>
>> Same as above.
>
> I would rather use `fromString(String text)` here, as using `Enum.valueOf()`
> does not work as smoothly as `FormatType`, since the `FormatStyle.text` value
> does not always match the `FormatStyle` enum name.
>
> For example, `FormatStyle.DEFAULT` has a `.text` value of "". So I would have
> to convert a "" to "default" to check for `FormatStyle.DEFAULT` via
> `Enum.valueOf()`, but at the same time, this also means that the behavior
> would now change to accept "default" as `FormatStyle.DEFAULT` (which is not
> in the spec).
>
> I could then add extra code to handle the "default" case, but I think at that
> point it would be much too much complexity compared to this `fromString()`
> method
Let's keep this static method then.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17663#discussion_r1476805391
PR Review Comment: https://git.openjdk.org/jdk/pull/17663#discussion_r1476805499
