On Fri, 14 Jul 2023 13:16:27 GMT, Pavel Rappo <pra...@openjdk.org> 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
> * java.nio.charset.Charset#compareTo seems **inconsistent** with equals. If
> so, I cannot see where that inconsistency is specified.
@AlanBateman, what do you think of that? For convenience, let me paste here
relevant source parts:
/**
...
* <p> Every charset has a <i>canonical name</i> and may also have one or
more
* <i>aliases</i>. The canonical name is returned by the {@link #name()
name} method
* of this class. Canonical names are, by convention, usually in upper
case.
* The aliases of a charset are returned by the {@link #aliases() aliases}
* method.
...
*/
public abstract class Charset implements Comparable<Charset>
{
...
/**
* Compares this charset to another.
*
* <p> Charsets are ordered by their canonical names, without regard to
* case.
...
*/
@Override
public final int compareTo(Charset that) {
return (name().compareToIgnoreCase(that.name()));
}
...
/**
* Tells whether or not this object is equal to another.
*
* <p> Two charsets are equal if, and only if, they have the same
canonical
* names. A charset is never equal to any other type of object. </p>
*
* @return {@code true} if, and only if, this charset is equal to the
* given object
*/
@Override
public final boolean equals(Object ob) {
if (this == ob)
return true;
return ob instanceof Charset other && name.equals(other.name());
}
...
}
Here's what Comparable has to say about this:
public interface Comparable<T> {
/**
...
* @apiNote
* It is strongly recommended, but <i>not</i> strictly required that
* {@code (x.compareTo(y)==0) == (x.equals(y))}. Generally speaking,
any
* class that implements the {@code Comparable} interface and violates
* this condition should clearly indicate this fact. The recommended
* language is "Note: this class has a natural ordering that is
* inconsistent with equals."
...
*/
public int compareTo(T o);
-------------
PR Comment: https://git.openjdk.org/jdk/pull/14886#issuecomment-1635967137
