RFR 8020156/8020009 TreeMap.values should report ORDERED/TreeMap.entrySet comparator should not be null

[Mike Duigou]

The changes look OK to me. I do prefer the later form but will accept either.

Mike

On Jul 25 2013, at 02:31 , Paul Sandoz wrote:

> Hi,
> 
> Please review the following patch that fixes two issues with TreeMap spliterators:
> 
>  http://cr.openjdk.java.net/~psandoz/tl/JDK-8020156-8020009-TreeMap/webrev/
> 
> It's unfortunate and damn ugly that i resorted to using raw types and a cast for the EntrySet Spliterator.getComparator method: 
> 
> +            // Since SORTED is reported and Map.Entry elements are not comparable
> +            // then a non-null comparator needs to be returned
> +            if (tree.comparator != null) {
> +                // Adapt the existing non-null comparator to compare entries
> +                // by key
> +                return Map.Entry.comparingByKey(tree.comparator);
> +            }
> +            else {
> +                // Return a comparator of entries by key, with K assumed to be
> +                // of Comparable
> +                // First obtain the Map.Entry.comparingByKey() comparator
> +                // as a raw type since K is not declared with
> +                // a upper bound of Comparable, then cast to the return type
> +                @SuppressWarnings("rawtypes")
> +                Comparator craw = Map.Entry.comparingByKey();
> +                @SuppressWarnings("unchecked")
> +                Comparator<? super Map.Entry<K, V>> c =
> +                        (Comparator<? super Map.Entry<K, V>>) craw;
> +                return c;
> +            }
> 
> 
> Perhaps rather than reusing a Map.Entry comparator method it is clearer to do the following:
> 
>                return (Comparator<? super Map.Entry<K, V>> & Serializable) (e1, e2) -> {
>                    @SuppressWarnings("unchecked")
>                    Comparable<? super K> k1 = (Comparable<? super K>) e1.getKey();
>                    return k1.compareTo(e2.getKey());
>                };
> 
> ? I marginally prefer that.
> 
> I created a new test java/util/Spliterators/SpliteratorCharacteristics that we can fill out over time for other collections as and when needed.
> 
> Paul.