Code Review Request Bug #7129185:(coll) Please add Collections.emptyNavigableSet() (original) (raw)

Darryl Mocek darryl.mocek at oracle.com
Tue Jan 24 18:51:27 UTC 2012

Jason,

see inline and updated webrev:

http://cr.openjdk.java.net/~dmocek/7129185/webrev.01

Darryl

On Tue 24 Jan 2012 09:54:52 AM PST, Jason Mehrens wrote:

Darryl, Here is my list of bugs: 1. The comparator method is using raw types. Should be Comparator<?_ _super E>. The webrev didn't pick up on this change. The method is this: public Comparator<? super E> comparator() { return null; } 2. The readResolve method comment is just wrong and the method implementation is redundant of the default behavior. readResolve has been removed. 3. What if I want to create an empty set navigable set with supplied comparator? Extending is not an option. This is the one issue I wanted to discuss...is this necessary? I was thinking about how this would be implemented. You would need to supply a comparator to the emptyNavigableSet. Other empty* methods don't take parameters and adding a method to supply a comparator would require an additional method.

Darryl

Here are my lists of RFEs: 5. Why not extend EmptySet so you don't have to reimplement the optimized methods? 6. Why not create a default access static final reference named EMPTYNAVIGABLESET inside of the EmptyNavigableSet and use that in readResolve and in Collections.emptyNavigableSet? That gives you the nice singleton behavior and on demand class loading. 7. CCE lacks a descriptive message that you normally get if you used Class.cast or just an implicit cast. Jason

> Date: Mon, 23 Jan 2012 15:19:01 -0800 > From: darryl.mocek at oracle.com > To: core-libs-dev at openjdk.java.net > Subject: Code Review Request Bug #7129185:(coll) Please add Collections.emptyNavigableSet() > > Re-sending this with the synopsis in the subject line (and the correct > bug #). > > Hello core-libs. Please review this patch to fix Bug #7129185. This > fix addresses comments made by Jason Mehrens to the commit of the fix > for bug #4533691, including adding a Collections.emptyNavigableSet > method. Tests are included. > > Webrev, can be found here: > http://cr.openjdk.java.net/~dmocek/7129185/webrev.00 > > Thanks, > Darryl

More information about the core-libs-dev mailing list