RFR 8009736: Comparator API cleanup

[Paul Sandoz]

On Jun 13, 2013, at 7:28 PM, Tom Hawtin <tom.hawtin at oracle.com> wrote:

> On 12/06/2013 17:36, Paul Sandoz wrote:
> 
>> "The returned comparator is serializable and does not permit null arguments when comparing objects".
> 
> Being Serializable at all is odd. It brings the previously 
> implementation-dependent name and fields into the API spec.
> 

That is, alas, inherently the nature of serialization. Collections are serializable, some collections take Comparator instances on construction (e.g. TreeSet).


> Serialisability also allows curious behaviour. Whilst not, say, a 
> vulnerability itself, oddities aren't generally a good idea. For an 
> example of strangeness, NullComparator can have a sign of 
> -Integer.MIN_VALUE. It can also have a `real` field of null which is 
> otherwise checked for during construction,

I dunno what to say except "serialization is bad! i wish it were never in the JDK" :-)


> although strangely the 
> compare method also checks whether real is null or not.
> 

Yes, Henry is gonna fix that null check.

FWIW i find this marginally clearer:

        NullComparator(boolean nullsFirst, Comparator<? super T> real) {
            this(nullsFirst ? -1 : 1, real);
        }

        private NullComparator(int sign, Comparator<? super T> real) {
            this.sign = sign;
            this.real = (Comparator<T>) Objects.requireNonNull(real);
        }

but it's just nit-picking.

Paul.