On Wed, 19 Nov 2025 15:43:09 GMT, Andy Goryachev <[email protected]> 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

Reply via email to