On 11/07/2011 01:28 AM, David Holmes wrote:
Hi Mike,

On 5/11/2011 2:29 AM, Mike Duigou wrote:

On Nov 3 2011, at 17:40 , David Holmes wrote:
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?

I certainly expected to see a response to the comments and to see definitive reviewer responses to the effect "I agree with this change" before it was pushed. Even if the only comment had been to drop EMPTY_SORTED_SET, I would have expected an email stating that this had been done and the amended changeset pushed. But in this case there was more to address.

More inline ...


On 31/10/2011 10:52 AM, David Holmes wrote:
On 29/10/2011 8:44 AM, Darryl Mocek wrote:
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.

Naively perhaps I would have expected the range on an empty set to be fully closed, rather than fully open.

The spec clearly says that headSet/tailSet creates subsets that should check one of their bounds
regardless if the master set is empty or not.



That aside it is unclear what this means from an API specification perspective. Is this difference in behaviour an error ie does it violate the spec?

Yes.

Is it simply inconsistent with other collections, but allowed by the spec?

No

Could we in fact "fix" it?

Yes

If we can why didn't we?

perhaps because it's weird to have a view of a stateless collection
which is not stateless.

The other solution is to relax the spec to say that the returned set
may be not a restricted and make the IAE optional.


Cheers,
David

Mike

Rémi

Reply via email to