RFR: 8250802: Refactor StringConverter and its subclasses [v4]

[Andy Goryachev]

On Wed, 19 Nov 2025 20:42:58 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

>> I meant to suggest using `this` when parameter name is the same as the field name.  Even though the compiler and IDE know which is which, the IDE might stumble during refactoring.
>> 
>> L50
>> 
>> chrono = Objects.requireNonNullElse(chrono, DEFAULT_CHRONO);
>
> I don't understand this either :)
> 
> What is the field name in L50?
> 
> This is the discussed code:
> 
>     // fields
>     private final DateTimeFormatter formatter;
>     private final DateTimeFormatter parser;
> 
>     // constructor
>     protected BaseTemporalStringConverter(FormatStyle dateStyle, FormatStyle timeStyle, Locale locale, Chronology chrono) {
>         locale = Objects.requireNonNullElse(locale, DEFAULT_LOCALE);        // local variable reassignment
>         chrono = Objects.requireNonNullElse(chrono, DEFAULT_CHRONO);        // local variable reassignment
>         formatter = createFormatter(dateStyle, timeStyle, locale, chrono);  // field assignment
>         parser = createParser(dateStyle, timeStyle, locale, chrono);        // field assignment
>     }
> 
> John is against local variable reassignment, which I understand, but I find it useful in the specific cases of "curating" like clamping and non-null-ing. It avoids mistakes like this:
> 
> void area(long length, long width, Shape shape) {
>     posLength = length <= 0 ? 1 : length;
>     posWidth = width <= 0 ? 1 : width;
> 
>     if (shape == TRIANGLE) {
>         return (double) (length * width) / 2;
>     } else if (shape == RECT) {
>         return posLength * posWidth;
>     }
> }
> 
> 
> John also wants `this` when doing field assignments (from what I understood), which one has to use if the parameter name is the same as the field (otherwise I don't think the IDE can know which is which). If it's unambiguous, I tend to avoid `this` when it's not required except for some cases such as uniformity.
> 
> I don't mind adding `this` to field assignments or renaming local variables, I just need to understand to consensus.

I have to be careful when reviewing in github - I thought it was a field.  chrono is not a field, does not need `this`.

Parameter reassignment is ok (in my opinion).

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1880#discussion_r2543526538