You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by te...@apache.org on 2010/07/21 16:56:05 UTC
svn commit: r966265 - in
/harmony/enhanced/java/trunk/classlib/modules/luni/src:
main/java/java/util/PriorityQueue.java
test/api/common/org/apache/harmony/luni/tests/java/util/PriorityQueueTest.java
Author: tellison
Date: Wed Jul 21 14:56:05 2010
New Revision: 966265
URL: http://svn.apache.org/viewvc?rev=966265&view=rev
Log:
Fix problems with PriorityQueue:
1 - the remove test should use equality using equals() not the result of the comparator being 0;
2 - attempting to remove an object of the 'wrong' type should just return false, not throw a class cast exception.
Modified:
harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/util/PriorityQueue.java
harmony/enhanced/java/trunk/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/PriorityQueueTest.java
Modified: harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/util/PriorityQueue.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/util/PriorityQueue.java?rev=966265&r1=966264&r2=966265&view=diff
==============================================================================
--- harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/util/PriorityQueue.java (original)
+++ harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/util/PriorityQueue.java Wed Jul 21 14:56:05 2010
@@ -239,22 +239,17 @@ public class PriorityQueue<E> extends Ab
* was not in the priority queue.
*/
@Override
- @SuppressWarnings("unchecked")
public boolean remove(Object o) {
- if (o == null) {
+ if (o == null || size == 0) {
return false;
}
- int targetIndex;
- for (targetIndex = 0; targetIndex < size; targetIndex++) {
- if (0 == this.compare((E) o, elements[targetIndex])) {
- break;
+ for (int i = 0; i < size; i++) {
+ if (o.equals(elements[i])) {
+ removeAt(i);
+ return true;
}
}
- if (size == 0 || size == targetIndex) {
- return false;
- }
- removeAt(targetIndex);
- return true;
+ return false;
}
/**
Modified: harmony/enhanced/java/trunk/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/PriorityQueueTest.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/java/trunk/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/PriorityQueueTest.java?rev=966265&r1=966264&r2=966265&view=diff
==============================================================================
--- harmony/enhanced/java/trunk/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/PriorityQueueTest.java (original)
+++ harmony/enhanced/java/trunk/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/PriorityQueueTest.java Wed Jul 21 14:56:05 2010
@@ -133,7 +133,18 @@ public class PriorityQueueTest extends T
for (int i = 0; i < integerQueue.size(); i++) {
assertEquals(newArray[i], integerQueue.poll());
}
+ }
+ public void test_iterator_removeEquals() {
+ PriorityQueue<String> integerQueue = new PriorityQueue<String>(10, new MockComparatorStringByLength());
+ String[] array = { "ONE", "TWO", "THREE", "FOUR", "FIVE" };
+ for (int i = 0; i < array.length; i++) {
+ integerQueue.offer(array[i]);
+ }
+ // Try removing an entry that the comparator says is equal
+ assertFalse(integerQueue.remove("123"));
+ assertFalse(integerQueue.remove("one"));
+ assertTrue(integerQueue.remove("THREE"));
}
/**
@@ -594,7 +605,7 @@ public class PriorityQueueTest extends T
queue.offer(array[i]);
}
assertFalse(queue.contains("BB"));
- assertTrue(queue.remove("BB"));
+ assertTrue(queue.remove("AA"));
}
/**
@@ -607,12 +618,7 @@ public class PriorityQueueTest extends T
PriorityQueue<Integer> integerQueue = new PriorityQueue<Integer>(list);
assertFalse(integerQueue.remove(111));
assertFalse(integerQueue.remove(null));
- try {
- integerQueue.remove("");
- fail("should throw ClassCastException");
- } catch (ClassCastException e) {
- // expected
- }
+ assertFalse(integerQueue.remove(""));
}
/**
@@ -634,13 +640,8 @@ public class PriorityQueueTest extends T
Integer[] array = { 2, 45, 7, -12, 9, 23, 17, 1118, 10, 16, 39 };
List<Integer> list = Arrays.asList(array);
PriorityQueue<Integer> integerQueue = new PriorityQueue<Integer>(list);
- try {
- integerQueue.remove(new Float(1.3F));
- fail("should throw ClassCastException");
- } catch (ClassCastException e) {
- // expected
- }
-
+ assertFalse(integerQueue.remove(new Float(1.3F)));
+
// although argument element type is not compatible with those in queue,
// but comparator supports it.
MockComparator<Object> comparator = new MockComparator<Object>();
@@ -652,12 +653,7 @@ public class PriorityQueueTest extends T
PriorityQueue<Object> queue = new PriorityQueue<Object>();
Object o = new Object();
queue.offer(o);
- try {
- queue.remove(o);
- fail("should throw ClassCastException");
- } catch (ClassCastException e) {
- // expected
- }
+ assertTrue(queue.remove(o));
}
/**
Re: svn commit: r966265 - in /harmony/enhanced/java/trunk/classlib/modules/luni/src:
main/java/java/util/PriorityQueue.java test/api/common/org/apache/harmony/luni/tests/java/util/PriorityQueueTest.java
Posted by Tim Ellison <t....@gmail.com>.
On 23/Jul/2010 08:54, Mark Hindess wrote:
> In message <20...@eris.apache.org>, tellison@apache.org
> writes:
>> Author: tellison
>> Date: Wed Jul 21 14:56:05 2010
>> New Revision: 966265
>>
>> URL: http://svn.apache.org/viewvc?rev=966265&view=rev
>> Log:
>> Fix problems with PriorityQueue:
>> 1 - the remove test should use equality using equals() not the result of the
>> comparator being 0;
>> 2 - attempting to remove an object of the 'wrong' type should just return
>> false, not throw a class cast exception.
>
> Tim,
>
> There is a non-trivial conflict in the merge of this commit from trunk
> to the java6 branch - due to the fact that this appears to have been
> fixed there already but with an equals method call the opposite way
> around.
>
> Would you mind doing the merge for this commit. The command will be
> something like:
>
> svn merge -c 966265 --accept postpone \
> https://svn.apache.org/repos/asf/harmony/enhanced/java/trunk
Done at r967015.
Regards,
Tim
Re: svn commit: r966265 - in /harmony/enhanced/java/trunk/classlib/modules/luni/src: main/java/java/util/PriorityQueue.java test/api/common/org/apache/harmony/luni/tests/java/util/PriorityQueueTest.java
Posted by Mark Hindess <ma...@googlemail.com>.
In message <20...@eris.apache.org>, tellison@apache.org
writes:
>
> Author: tellison
> Date: Wed Jul 21 14:56:05 2010
> New Revision: 966265
>
> URL: http://svn.apache.org/viewvc?rev=966265&view=rev
> Log:
> Fix problems with PriorityQueue:
> 1 - the remove test should use equality using equals() not the result of the
> comparator being 0;
> 2 - attempting to remove an object of the 'wrong' type should just return
> false, not throw a class cast exception.
Tim,
There is a non-trivial conflict in the merge of this commit from trunk
to the java6 branch - due to the fact that this appears to have been
fixed there already but with an equals method call the opposite way
around.
Would you mind doing the merge for this commit. The command will be
something like:
svn merge -c 966265 --accept postpone \
https://svn.apache.org/repos/asf/harmony/enhanced/java/trunk
Regards,
Mark.