Code Review Request for 4533691 (add Collections.EMPTY_SORTED_SET)

[Mike Duigou]

On Nov 3 2011, at 17:40 , David Holmes wrote:

> Mike,
> 
> I see that you have pushed a version of this change and that I was listed as a reviewer. However I never saw an updated webrev and there was no response to my query below. In fact I never saw any response to any of the reviewers comments on this.

I missed your question about the range on an empty set. My comments below.

Removing EMPTY_SORTED_SET was the only other comment to my knowledge. Darryl adapted the patch and EMPTY_SORTED_SET was not part of the commit. Since the change was removal of a newly proposed field an additional review cycle wasn't thought to be necessary. Perhaps incorrectly?

Mike

> David
> 
> On 31/10/2011 10:52 AM, David Holmes wrote:
>> Darryl,
>> 
>> On 29/10/2011 8:44 AM, Darryl Mocek wrote:
>>> Hello. Please review this patch to add empty sorted set to the
>>> Collections class. Test case provided.
>>> 
>>> Webrev: http://cr.openjdk.java.net/~mduigou/4533691/1/webrev/
>> 
>> This was an RFE from 2001 - pre-generics. I don't think it makes sense
>> to add to the pre-generics pollution by defining EMPTY_SORTED_SET.
>> 
>>> Additional Notes to Reviewers:
>>> The sets resulting from tailSet() headSet() and subSet() normally
>>> include the range which was used to create them. Using these methods
>>> with emptySortedSet() does not currently set a range on the resulting
>>> sets.
>> 
>> What is the range on an empty set?

The results of Collections.emptySortedSet() will behave differently than Collections.unmodifiableSortedSet(new TreeSet()) for cases involving headSet, tailSet and subSet. These three operations remember the range used to create the sub set in addition to providing the elements within that range. The range is considered in a few cases such add item addition (added items must be within the range of the subset) and any subset created from a range sub set must lie within the range. Relevant to emptySortedSet is the creation of sub sets off of a ranged sub set.

public static void main(String[] args) {
    SortedSet<Double> foo = Collections.unmodifiableSortedSet(new TreeSet<Double>());

    SortedSet<Double> two = foo.headSet(Double.valueOf(2.0));  // range < 2.0
    SortedSet<Double> one = two.headSet(Double.valueOf(1.0));
    try {
        SortedSet<Double> three = two.headSet(Double.valueOf(3.0)); // IAE "toKey out of range"
    } catch (IllegalArgumentException expected) {
        expected.printStackTrace(System.err);
    }

    two = foo.tailSet(Double.valueOf(2.0)); // range >= 2.0 
    try {
        one = two.tailSet(Double.valueOf(1.0));  // IAE "fromKey out of range"
    } catch (IllegalArgumentException expected) {
        expected.printStackTrace(System.err);
    }
    SortedSet<Double> three = two.tailSet(Double.valueOf(3.0));

    two = foo.subSet(Double.valueOf(2.0),Double.valueOf(3.0)); // 2.0 <= range < 3.0
    try {
        one = two.subSet(Double.valueOf(1.0),Double.valueOf(2.0));  // IAE "fromKey out of range"
    } catch (IllegalArgumentException expected) {
        expected.printStackTrace(System.err);
    }
    three = two.subSet(Double.valueOf(2.0),Double.valueOf(3.0));
}

This program throws three IAE exceptions. Replacing the first statement with:

SortedSet<Double> foo = Collections.emptySortedSet();

would result in no IAE exceptions because they emptySortedSet() does not track ranges for sub sets derived with headSet(), tailSet() and subSet(). The range on emptySortedSet and derived sets is always fully open.

Mike