Fixed BinaryHeap / BinaryBuffer remove(object) bug.

PR #25818
Reported by: Steve Phelps


git-svn-id: https://svn.apache.org/repos/asf/jakarta/commons/proper/collections/trunk@131494 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Phil Steitz 2004-01-01 23:56:51 +00:00
parent 6f705a723c
commit 6c12ca3818
4 changed files with 334 additions and 87 deletions

View File

@ -1,10 +1,10 @@
/*
* $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/java/org/apache/commons/collections/BinaryHeap.java,v 1.16 2004/01/01 19:00:20 scolebourne Exp $
* $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/java/org/apache/commons/collections/BinaryHeap.java,v 1.17 2004/01/01 23:56:51 psteitz Exp $
* ====================================================================
*
* The Apache Software License, Version 1.1
*
* Copyright (c) 2001-2003 The Apache Software Foundation. All rights
* Copyright (c) 2001-2004 The Apache Software Foundation. All rights
* reserved.
*
* Redistribution and use in source and binary forms, with or without
@ -92,7 +92,7 @@ import java.util.NoSuchElementException;
* </pre>
*
* @since Commons Collections 1.0
* @version $Revision: 1.16 $ $Date: 2004/01/01 19:00:20 $
* @version $Revision: 1.17 $ $Date: 2004/01/01 23:56:51 $
*
* @author Peter Donald
* @author Ram Chidambaram
@ -319,8 +319,9 @@ public final class BinaryHeap extends AbstractCollection
}
/**
* Percolates element down heap from top.
* Assume it is a maximum heap.
* Percolates element down heap from the position given by the index.
* <p>
* Assumes it is a mimimum heap.
*
* @param index the index for the element
*/
@ -350,8 +351,9 @@ public final class BinaryHeap extends AbstractCollection
}
/**
* Percolates element down heap from top.
* Assume it is a maximum heap.
* Percolates element down heap from the position given by the index.
* <p>
* Assumes it is a maximum heap.
*
* @param index the index of the element
*/
@ -381,16 +383,15 @@ public final class BinaryHeap extends AbstractCollection
}
/**
* Percolates element up heap from bottom.
* Assume it is a maximum heap.
* Percolates element up heap from the position given by the index.
* <p>
* Assumes it is a minimum heap.
*
* @param element the element
* @param index the index of the element to be percolated up
*/
protected void percolateUpMinHeap(final Object element) {
int hole = ++m_size;
m_elements[hole] = element;
protected void percolateUpMinHeap(final int index) {
int hole = index;
Object element = m_elements[hole];
while (hole > 1 && compare(element, m_elements[hole / 2]) < 0) {
// save element that is being pushed down
// as the element "bubble" is percolated up
@ -398,19 +399,32 @@ public final class BinaryHeap extends AbstractCollection
m_elements[hole] = m_elements[next];
hole = next;
}
m_elements[hole] = element;
}
/**
* Percolates element up heap from bottom.
* Percolates a new element up heap from the bottom.
* <p>
* Assumes it is a minimum heap.
*
* @param element the element
*/
protected void percolateUpMinHeap(final Object element) {
m_elements[++m_size] = element;
percolateUpMinHeap(m_size);
}
/**
* Percolates element up heap from from the position given by the index.
* <p>
* Assume it is a maximum heap.
*
* @param element the element
*/
protected void percolateUpMaxHeap(final Object element) {
int hole = ++m_size;
protected void percolateUpMaxHeap(final int index) {
int hole = index;
Object element = m_elements[hole];
while (hole > 1 && compare(element, m_elements[hole / 2]) > 0) {
// save element that is being pushed down
// as the element "bubble" is percolated up
@ -422,6 +436,18 @@ public final class BinaryHeap extends AbstractCollection
m_elements[hole] = element;
}
/**
* Percolates a new element up heap from the bottom.
* <p>
* Assume it is a maximum heap.
*
* @param element the element
*/
protected void percolateUpMaxHeap(final Object element) {
m_elements[++m_size] = element;
percolateUpMaxHeap(m_size);
}
/**
* Compares two objects using the comparator if specified, or the
* natural order otherwise.
@ -494,18 +520,34 @@ public final class BinaryHeap extends AbstractCollection
}
public void remove() {
if (lastReturnedIndex == -1) throw new IllegalStateException();
if (lastReturnedIndex == -1) {
throw new IllegalStateException();
}
m_elements[ lastReturnedIndex ] = m_elements[ m_size ];
m_elements[ m_size ] = null;
m_size--;
if( m_size != 0 )
{
//percolate top element to it's place in tree
if( m_isMinHeap ) percolateDownMinHeap( lastReturnedIndex );
else percolateDownMaxHeap( lastReturnedIndex );
m_size--;
if( m_size != 0 && lastReturnedIndex <= m_size) {
int compareToParent = 0;
if (lastReturnedIndex > 1) {
compareToParent = compare(m_elements[lastReturnedIndex],
m_elements[lastReturnedIndex / 2]);
}
if (m_isMinHeap) {
if (lastReturnedIndex > 1 && compareToParent < 0) {
percolateUpMinHeap(lastReturnedIndex);
} else {
percolateDownMinHeap(lastReturnedIndex);
}
} else { // max heap
if (lastReturnedIndex > 1 && compareToParent > 0) {
percolateUpMaxHeap(lastReturnedIndex);
} else {
percolateDownMaxHeap(lastReturnedIndex);
}
}
}
index--;
lastReturnedIndex = -1;
lastReturnedIndex = -1;
}
};

View File

@ -1,10 +1,10 @@
/*
* $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/java/org/apache/commons/collections/buffer/Attic/BinaryBuffer.java,v 1.1 2004/01/01 19:01:34 scolebourne Exp $
* $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/java/org/apache/commons/collections/buffer/Attic/BinaryBuffer.java,v 1.2 2004/01/01 23:56:51 psteitz Exp $
* ====================================================================
*
* The Apache Software License, Version 1.1
*
* Copyright (c) 2001-2003 The Apache Software Foundation. All rights
* Copyright (c) 2001-2004 The Apache Software Foundation. All rights
* reserved.
*
* Redistribution and use in source and binary forms, with or without
@ -92,7 +92,7 @@ import org.apache.commons.collections.BufferUnderflowException;
* </pre>
*
* @since Commons Collections 3.0 (previously BinaryHeap v1.0)
* @version $Revision: 1.1 $ $Date: 2004/01/01 19:01:34 $
* @version $Revision: 1.2 $ $Date: 2004/01/01 23:56:51 $
*
* @author Peter Donald
* @author Ram Chidambaram
@ -337,9 +337,11 @@ public class BinaryBuffer extends AbstractCollection implements Buffer {
return elements.length == size + 1;
}
/**
* Percolates element down heap from top.
* Assume it is a minimum heap.
* Percolates element down heap from the position given by the index.
* <p>
* Assumes it is a mimimum heap.
*
* @param index the index for the element
*/
@ -369,8 +371,9 @@ public class BinaryBuffer extends AbstractCollection implements Buffer {
}
/**
* Percolates element down heap from top.
* Assume it is a maximum heap.
* Percolates element down heap from the position given by the index.
* <p>
* Assumes it is a maximum heap.
*
* @param index the index of the element
*/
@ -400,17 +403,49 @@ public class BinaryBuffer extends AbstractCollection implements Buffer {
}
/**
* Percolates element up heap from bottom.
* Assume it is a minimum heap.
* Percolates element up heap from the position given by the index.
* <p>
* Assumes it is a minimum heap.
*
* @param index the index of the element to be percolated up
*/
protected void percolateUpMinHeap(final int index) {
int hole = index;
Object element = elements[hole];
while (hole > 1 && compare(element, elements[hole / 2]) < 0) {
// save element that is being pushed down
// as the element "bubble" is percolated up
final int next = hole / 2;
elements[hole] = elements[next];
hole = next;
}
elements[hole] = element;
}
/**
* Percolates a new element up heap from the bottom.
* <p>
* Assumes it is a minimum heap.
*
* @param element the element
*/
protected void percolateUpMinHeap(final Object element) {
int hole = ++size;
elements[++size] = element;
percolateUpMinHeap(size);
}
elements[hole] = element;
/**
* Percolates element up heap from from the position given by the index.
* <p>
* Assume it is a maximum heap.
*
* @param element the element
*/
protected void percolateUpMaxHeap(final int index) {
int hole = index;
Object element = elements[hole];
while (hole > 1 && compare(element, elements[hole / 2]) < 0) {
while (hole > 1 && compare(element, elements[hole / 2]) > 0) {
// save element that is being pushed down
// as the element "bubble" is percolated up
final int next = hole / 2;
@ -422,23 +457,15 @@ public class BinaryBuffer extends AbstractCollection implements Buffer {
}
/**
* Percolates element up heap from bottom.
* Percolates a new element up heap from the bottom.
* <p>
* Assume it is a maximum heap.
*
* @param element the element
*/
protected void percolateUpMaxHeap(final Object element) {
int hole = ++size;
while (hole > 1 && compare(element, elements[hole / 2]) > 0) {
// save element that is being pushed down
// as the element "bubble" is percolated up
final int next = hole / 2;
elements[hole] = elements[next];
hole = next;
}
elements[hole] = element;
elements[++size] = element;
percolateUpMaxHeap(size);
}
/**
@ -495,19 +522,31 @@ public class BinaryBuffer extends AbstractCollection implements Buffer {
if (lastReturnedIndex == -1) {
throw new IllegalStateException();
}
elements[lastReturnedIndex] = elements[size];
elements[size] = null;
size--;
if (size != 0) {
//percolate top element to it's place in tree
if (ascendingOrder) {
percolateDownMinHeap(lastReturnedIndex);
} else {
percolateDownMaxHeap(lastReturnedIndex);
elements[ lastReturnedIndex ] = elements[ size ];
elements[ size ] = null;
size--;
if( size != 0 && lastReturnedIndex <= size) {
int compareToParent = 0;
if (lastReturnedIndex > 1) {
compareToParent = compare(elements[lastReturnedIndex],
elements[lastReturnedIndex / 2]);
}
if (ascendingOrder) {
if (lastReturnedIndex > 1 && compareToParent < 0) {
percolateUpMinHeap(lastReturnedIndex);
} else {
percolateDownMinHeap(lastReturnedIndex);
}
} else { // max heap
if (lastReturnedIndex > 1 && compareToParent > 0) {
percolateUpMaxHeap(lastReturnedIndex);
} else {
percolateDownMaxHeap(lastReturnedIndex);
}
}
}
index--;
lastReturnedIndex = -1;
lastReturnedIndex = -1;
}
};

View File

@ -1,10 +1,10 @@
/*
* $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/test/org/apache/commons/collections/TestBinaryHeap.java,v 1.13 2003/11/18 22:37:16 scolebourne Exp $
* $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/test/org/apache/commons/collections/TestBinaryHeap.java,v 1.14 2004/01/01 23:56:51 psteitz Exp $
* ====================================================================
*
* The Apache Software License, Version 1.1
*
* Copyright (c) 2001-2003 The Apache Software Foundation. All rights
* Copyright (c) 2001-2004 The Apache Software Foundation. All rights
* reserved.
*
* Redistribution and use in source and binary forms, with or without
@ -62,6 +62,7 @@ import java.util.Arrays;
import java.util.Collection;
import java.util.Comparator;
import java.util.NoSuchElementException;
import java.util.Random;
import junit.framework.Test;
import junit.framework.TestSuite;
@ -73,7 +74,7 @@ import org.apache.commons.collections.comparators.ReverseComparator;
/**
* Tests the BinaryHeap.
*
* @version $Revision: 1.13 $ $Date: 2003/11/18 22:37:16 $
* @version $Revision: 1.14 $ $Date: 2004/01/01 23:56:51 $
*
* @author Michael A. Smith
*/
@ -288,5 +289,97 @@ public class TestBinaryHeap extends AbstractTestCollection {
// expected
}
}
/**
* Illustrates bad internal heap state reported in Bugzilla PR #235818.
*/
public void testAddRemove() {
resetEmpty();
BinaryHeap heap = (BinaryHeap) collection;
heap.add(new Integer(0));
heap.add(new Integer(2));
heap.add(new Integer(4));
heap.add(new Integer(3));
heap.add(new Integer(8));
heap.add(new Integer(10));
heap.add(new Integer(12));
heap.add(new Integer(3));
confirmed.addAll(heap);
// System.out.println(heap);
Object obj = new Integer(10);
heap.remove(obj);
confirmed.remove(obj);
// System.out.println(heap);
verify();
}
/**
* Generate heaps staring with Integers from 0 - heapSize - 1.
* Then perform random add / remove operations, checking
* heap order after modifications. Alternates minHeaps, maxHeaps.
*
* Based on code provided by Steve Phelps in PR #25818
*
*/
public void testRandom() {
int iterations = 500;
int heapSize = 100;
int operations = 20;
Random randGenerator = new Random();
BinaryHeap h = null;
for(int i=0; i < iterations; i++) {
if (i < iterations / 2) {
h = new BinaryHeap(true);
} else {
h = new BinaryHeap(false);
}
for(int r = 0; r < heapSize; r++) {
h.add( new Integer( randGenerator.nextInt(heapSize)) );
}
for( int r = 0; r < operations; r++ ) {
h.remove(new Integer(r));
h.add(new Integer(randGenerator.nextInt(heapSize)));
}
checkOrder(h);
}
}
/**
* Pops all elements from the heap and verifies that the elements come off
* in the correct order. NOTE: this method empties the heap.
*/
protected void checkOrder(BinaryHeap h) {
Integer lastNum = null;
Integer num = null;
boolean fail = false;
while (!h.isEmpty()) {
num = (Integer) h.pop();
if (h.m_isMinHeap) {
assertTrue(lastNum == null || num.intValue() >= lastNum.intValue());
} else { // max heap
assertTrue(lastNum == null || num.intValue() <= lastNum.intValue());
}
lastNum = num;
num = null;
}
}
/**
* Returns a string showing the contents of the heap formatted as a tree.
* Makes no attempt at padding levels or handling wrapping.
*/
protected String showTree(BinaryHeap h) {
int count = 1;
StringBuffer buffer = new StringBuffer();
for (int offset = 1; count < h.size() + 1; offset *= 2) {
for (int i = offset; i < offset * 2; i++) {
if (i < h.m_elements.length && h.m_elements[i] != null)
buffer.append(h.m_elements[i] + " ");
count++;
}
buffer.append('\n');
}
return buffer.toString();
}
}

View File

@ -1,10 +1,10 @@
/*
* $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/test/org/apache/commons/collections/buffer/Attic/TestBinaryBuffer.java,v 1.1 2004/01/01 19:01:34 scolebourne Exp $
* $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/test/org/apache/commons/collections/buffer/Attic/TestBinaryBuffer.java,v 1.2 2004/01/01 23:56:51 psteitz Exp $
* ====================================================================
*
* The Apache Software License, Version 1.1
*
* Copyright (c) 2001-2003 The Apache Software Foundation. All rights
* Copyright (c) 2001-2004 The Apache Software Foundation. All rights
* reserved.
*
* Redistribution and use in source and binary forms, with or without
@ -61,6 +61,7 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Comparator;
import java.util.Random;
import junit.framework.Test;
import junit.framework.TestSuite;
@ -75,7 +76,7 @@ import org.apache.commons.collections.comparators.ReverseComparator;
/**
* Tests the BinaryHeap.
*
* @version $Revision: 1.1 $ $Date: 2004/01/01 19:01:34 $
* @version $Revision: 1.2 $ $Date: 2004/01/01 23:56:51 $
*
* @author Michael A. Smith
*/
@ -286,24 +287,96 @@ public class TestBinaryBuffer extends AbstractTestCollection {
} catch (BufferUnderflowException ex) {}
}
// public void testAddRemove() {
// resetEmpty();
// BinaryBuffer heap = (BinaryBuffer) collection;
// heap.add(new Integer(0));
// heap.add(new Integer(2));
// heap.add(new Integer(4));
// heap.add(new Integer(3));
// heap.add(new Integer(8));
// heap.add(new Integer(10));
// heap.add(new Integer(12));
// heap.add(new Integer(3));
// confirmed.addAll(heap);
// System.out.println(heap);
// Object obj = new Integer(10);
// heap.remove(obj);
// confirmed.remove(obj);
// System.out.println(heap);
// verify();
// }
/**
* Illustrates bad internal heap state reported in Bugzilla PR #235818.
*/
public void testAddRemove() {
resetEmpty();
BinaryBuffer heap = (BinaryBuffer) collection;
heap.add(new Integer(0));
heap.add(new Integer(2));
heap.add(new Integer(4));
heap.add(new Integer(3));
heap.add(new Integer(8));
heap.add(new Integer(10));
heap.add(new Integer(12));
heap.add(new Integer(3));
confirmed.addAll(heap);
// System.out.println(heap);
Object obj = new Integer(10);
heap.remove(obj);
confirmed.remove(obj);
// System.out.println(heap);
verify();
}
/**
* Generate heaps staring with Integers from 0 - heapSize - 1.
* Then perform random add / remove operations, checking
* heap order after modifications. Alternates minHeaps, maxHeaps.
*
* Based on code provided by Steve Phelps in PR #25818
*
*/
public void testRandom() {
int iterations = 500;
int heapSize = 100;
int operations = 20;
Random randGenerator = new Random();
BinaryBuffer h = null;
for(int i=0; i < iterations; i++) {
if (i < iterations / 2) {
h = new BinaryBuffer(true);
} else {
h = new BinaryBuffer(false);
}
for(int r = 0; r < heapSize; r++) {
h.add( new Integer( randGenerator.nextInt(heapSize)) );
}
for( int r = 0; r < operations; r++ ) {
h.remove(new Integer(r));
h.add(new Integer(randGenerator.nextInt(heapSize)));
}
checkOrder(h);
}
}
/**
* Pops all elements from the heap and verifies that the elements come off
* in the correct order. NOTE: this method empties the heap.
*/
protected void checkOrder(BinaryBuffer h) {
Integer lastNum = null;
Integer num = null;
boolean fail = false;
while (!h.isEmpty()) {
num = (Integer) h.remove();
if (h.ascendingOrder) {
assertTrue(lastNum == null || num.intValue() >= lastNum.intValue());
} else { // max heap
assertTrue(lastNum == null || num.intValue() <= lastNum.intValue());
}
lastNum = num;
num = null;
}
}
/**
* Returns a string showing the contents of the heap formatted as a tree.
* Makes no attempt at padding levels or handling wrapping.
*/
protected String showTree(BinaryBuffer h) {
int count = 1;
StringBuffer buffer = new StringBuffer();
for (int offset = 1; count < h.size() + 1; offset *= 2) {
for (int i = offset; i < offset * 2; i++) {
if (i < h.elements.length && h.elements[i] != null)
buffer.append(h.elements[i] + " ");
count++;
}
buffer.append('\n');
}
return buffer.toString();
}
}