Darryl, >> 2. The comparator method is using raw types. >The SortedSet.comparator() method spec allows returning of null. Right. But, the return type of the implementation is a raw comparator not Comparator<? super E> defined by the interface. >> 4. Only the IAE if statement is need for your range checks. NPE and CCE >> will occur in that if statement by default. CCE lacks a descriptive message >> the you get if you used Class.cast or just an implicit cast. >True. I'm trying to be clear in the code that the parameters are checked for >these things. Is this an optimization/style issue? What is the preference >here? My thought is that implicit code is more compact and produces a more descriptive error message on CCE. >> 5. What if I want to create an empty set sorted set with supplied comparator? >Extend EmptySortedSet and override the comparator method. I believe most uses >of EmptySortedSet will not want to supply their own Comparator. I can support >a suppliable Comparator if there's enough interest.
Extending is not an option because EmptySortedSet is private class, as it should be. Since the EmptySortedSet is serializable, adding support later on gets ugly. >>6. Why not implement an EmptyNavigableSet instead since the bug was entered >>before 1.6? >Is NavigableSet preferrable to SortedSet? There is currently no request for >EmptyNavigableSet, just EmptySortedSet. My understanding is that NavigableSet is preferred to SortedSet. See: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6415641 Jason > Subject: Re: Code Review Request for 4533691 (add > Collections.EMPTY_SORTED_SET) > From: mike.dui...@oracle.com > Date: Fri, 4 Nov 2011 09:29:09 -0700 > To: david.hol...@oracle.com > CC: core-libs-dev@openjdk.java.net > > > 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?
