On Wed, 19 Nov 2025 20:42:58 GMT, Nir Lisker <[email protected]> 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
