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