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