<i18n dev> RFR: 8263668: Update java.time to use instanceof pattern variable [v4]

[Roger Riggs]

On Mon, 19 Apr 2021 10:38:17 GMT, Patrick Concannon <pconcannon at openjdk.org> wrote:

>> Hi,
>> 
>> Could someone please review my code for updating the code in the `java.time` package to make use of the `instanceof` pattern variable?
>> 
>> Kind regards,
>> Patrick
>
> 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

I generally concur with `forax` about avoiding the use of single letter variables.

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.

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)`.

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

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