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.
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? Is it simply inconsistent with other collections, but allowed
by the spec? Could we in fact "fix" it? If we can why didn't we?
Cheers,
David
Mike
