On Mon, 19 Apr 2021 15:37:24 GMT, Roger Riggs <rri...@openjdk.org> wrote:

>> Patrick Concannon has updated the pull request with a new target base due to 
>> a merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains six additional 
>> commits since the last revision:
>> 
>>  - Merge remote-tracking branch 'origin/master' into JDK-8263668
>>  - Merge remote-tracking branch 'origin/master' into JDK-8263668
>>  - Merge remote-tracking branch 'origin/master' into JDK-8263668
>>  - Merge remote-tracking branch 'origin/master' into JDK-8263668
>>  - Merge remote-tracking branch 'origin/master' into JDK-8263668
>>  - 8263668: Update java.time to use instanceof pattern variable
>
> src/java.base/share/classes/java/time/Duration.java line 1435:
> 
>> 1433:                 && this.seconds == other.seconds
>> 1434:                 && this.nanos == other.nanos;
>> 1435:     }
> 
> Perhaps rename the argument and use `otherDuration` as the refinement.
> Otherwise, an inconsistency across various classes will creep in where the 
> more specific variable has a more general name.  In this case, the argument 
> type is Object, so the argument name `otherDuration` is not strictly true.

Parameter and pattern variable names swapped, as suggested. See 647bd6b

> src/java.base/share/classes/java/time/Instant.java line 1306:
> 
>> 1304:                 && this.seconds == other.seconds
>> 1305:                 && this.nanos == other.nanos;
>> 1306:     }
> 
> Ditto, `otherInstance` is not strictly always an instant and it would be more 
> consistent to swap the names.
> `(other instanceof Instant otherInstant)`.

Parameter and pattern variable names swapped, as suggested. See 647bd6b

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

PR: https://git.openjdk.java.net/jdk/pull/3170

Reply via email to