JDK-6268068 : (coll) PriorityQueue.remove(Object) should use equals, not its ordering, to find element to remove
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.util:collections
  • Affected Version: 6
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2005-05-10
  • Updated: 2021-03-11
  • Resolved: 2005-09-04
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 6
6 b51Fixed
Related Reports
Relates :  
Description
PriorityQueue.remove(Object) uses its ordering, that is,
either (Comparable)o.compareTo(e), or comparator.compare(o,e).

Instead it should use o.equals(e)... because:

- that's what the base interface Collection.remove(Object) spec says
- the implementation throws ClassCastException, but the spec does not
  mention this exception.
- remove(Object) should be consistent with contains(Object), i.e.
  remove(Object) should be able to find an element to remove iff
  contains(Object) is able to find it.  remove(Object) should be
  implemented in terms of equals iff contains(Object) is.

###@###.### 2005-05-10 17:43:47 GMT

Comments
An additional bit of explanation: PriorityQueue uses its comparison method to maintain order but NOT to determine membership. PQ can contain "duplicates," that is, multiple elements that compare to zero, unlike SortedSet/NavigableSet. Since PQ.contains() uses equals() to determine whether an object is a member, PQ.remove() should do the same -- as was done here.
11-03-2021

EVALUATION A fine idea. ###@###.### 2005-05-10 17:43:47 GMT
10-05-2005

SUGGESTED FIX private int indexOf(Object o) { if (o == null) return -1; for (int i = 1; i <= size; i++) if (o.equals(queue[i])) return i; return -1; } /** * Removes a single instance of the specified element from this queue, * if it is present. More formally, removes an element <tt>e</tt> such * that <tt>o.equals(e)</tt>, if this queue contains one or more such * elements. Returns true if this queue contained the specified element * (or equivalently, if this queue changed as a result of the call). * * @param o element to be removed from this queue, if present * @return <tt>true</tt> if this queue changed as a result of the call */ public boolean remove(Object o) { int i = indexOf(o); if (i == -1) return false; else { removeAt(i); return true; } } /** * Returns <tt>true</tt> if this queue contains the specified element. * More formally, returns <tt>true</tt> if and only if this queue contains * at least one element <tt>e</tt> such that <tt>o.equals(e)</tt>. * * @param o object to be checked for containment in this queue * @return <tt>true</tt> if this queue contains the specified element */ public boolean contains(Object o) { return indexOf(o) != -1; } ###@###.### 2005-05-10 17:43:47 GMT
10-05-2005