On Fri, 14 Jul 2023 13:16:27 GMT, Pavel Rappo <[email protected]> wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> equals, hashCode, and compareTo for in java.nio and implementation code.
>>
>> Please note, test results are pending.
>>
>> Additional notes:
>>
>> * This PR saves a volatile read in
>> java.nio.file.attribute.AclEntry.hashCode. Not that it's too important, but
>> worth noting because of rearrangements.
>>
>> * java.nio.charset.Charset#compareTo seems **inconsistent** with equals. If
>> so, I cannot see where that inconsistency is specified.
>>
>> * Is this a **bug** in sun.nio.ch.FileKey#hashCode? Tell me if not, I'll
>> revert it.
>>
>> * This PR simplifies the tail of java.nio.file.attribute.FileTime.compareTo.
>> Unless I'm missing something, that comment in source above the affected
>> lines **seems** not to prohibit such a simplification.
>>
>> * sun.nio.fs.UnixFileStore#hashCode does not include entry.name(). While
>> it's not wrong, I wonder if it was on purpose.
>>
>> * Despite its title, this PR also and opportunistically refactors
>> sun.nio.fs.UnixPath.endsWith.
>
> Pavel Rappo has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Address another case from feedback
> - Address feedback
src/java.base/share/classes/java/nio/charset/Charset.java line 987:
> 985:
> 986: /**
> 987: * {@return the string describing this charset}
You've changed this to "the string", which hints of ==, I think it should be
reverted to a "a string".
src/java.base/unix/classes/sun/nio/fs/UnixPath.java line 713:
> 711:
> 712: return Arrays.equals(this.path, thisPos, thisLen, that.path,
> thatPos,
> 713: thatLen);
My comment here was "thatLen" ended up on its own line, it can go after thatPos
without making it too long.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14886#discussion_r1263932426
PR Review Comment: https://git.openjdk.org/jdk/pull/14886#discussion_r1263936625
