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

[Nir Lisker]

On Wed, 19 Nov 2025 15:43:09 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>>> I second John's suggestion with `this.`. It matters when refactoring, even with an IDE.
>> 
>> I'm not sure what you want me to change here exactly. Is it about renaming parameters (see [this discussion](https://github.com/openjdk/jfx/pull/1880#discussion_r2297668458)), or not using `this`, which you didn't seem to mind [here](https://github.com/openjdk/jfx/pull/1880#discussion_r2297721228)?
>
> 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.

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

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