Reputation: 285
I wrote this method to test my priority queue, which has as parameter an ArrayList and a Comparator:
@Test
public void testPriorityQueue_ExtractMax() {
PriorityQueue queue = new PriorityQueue(new IntegerComparator());
Integer[] arrayExp={14, 9};
queue.insert(16);
queue.insert(9);
queue.insert(14);
queue.extractMax();
assertArrayEquals(arrayExp, queue.getArray().toArray());
}
Now, if I execute it, it says that the first elements in my result is 9, but I has to be 14 (the new root of my queue). These are the methods extract max and heapify. How can I solve it?
public void heapify(int index) {
int largest = index;
int leftIndex = 2 * index + 1;
int rightIndex = 2 * index + 2;
if (leftIndex < queue.size() && c.compare(queue.get(index), (queue.get(leftIndex))) < 0)
largest = leftIndex;
if (rightIndex < queue.size() && c.compare(queue.get(largest), queue.get(rightIndex)) < 0)
largest = rightIndex;
if (largest != index) {
swap(index, largest);
heapify(largest);
}
}
public T extractMax() {
if (queue.size() == 0) return null;
T min = queue.get(0);
queue.remove(0);
queue.set(0, queue.get(queue.size() - 1));
heapify(0);
return min;
}
This is the IntegerComparator:
public class IntegerComparator implements Comparator<Integer>{
@Override
public int compare(Integer l1, Integer l2) {
int result = l1.compareTo(l2);
if(result != 0)
return result;
return -1;
}
}
EDIT_2: This is my insert method, can it be the problem:
public void insert(T elem) {
int i = queue.size();
int parentIndex = (i - 1) / 2;
while (i > 0 && c.compare(elem, queue.get(parentIndex)) == 1) {
queue.set(i, queue.get(parentIndex));
i = parentIndex;
parentIndex = (i - 1) / 2;
}
queue.add(i, elem);
}
Upvotes: 2
Views: 2520
Reputation: 3225
When you do this queue.remove(i);
automatically shift all elements, after index i, to left with one position.
https://www.tutorialspoint.com/java/util/arraylist_remove.htm
And with queue.set(0, queue.get(queue.size() - 1));
You just set value of last index from queue on index 0 and lose value from index 0, but it still remain on last position.
After reading insert method
At queue.set(i, queue.get(parentIndex));
if i = queue.size()
in first step, that mean index i don't exist in queue.
Upvotes: 1
Reputation: 73
As a suggestion, consider adding an unused element at index 0 so that accessing the parents/children of each node is more intuitive.
Example:
Consider the heap heap = [-1, 6, 4, 5, 2, 1, 4, 3]
where the root is defined as heap[1]
, and the data for the heap is located from index = 1
to the heap's size.
To access the children of a node at index
, it is intuitive to say that the left child is defined as heap[2 * index]
and the right child is defined as heap[2 * index + 1]
. Similarly, to access the parent node of a node at index
, one can use int truncation to access parents:
int parentInd = (int)(index/2);
T parent = heap[parentInd];
Solution:
As raul1ro has pointed out, you are losing the data from index 0 that you were not intending to remove.
In extractMax()
:
T min = queue.get(0);
queue.remove(0);
queue.set(0, queue.get(queue.size() - 1));
should be:
T min = queue.get(0); //Get min
T temp = queue.get(queue.size() - 1); //Get the last element in heap as temp
queue.remove(queue.size - 1); //Remove the last element
queue.set(0, temp); //Set the root to the temp value so that you can heapify
This will make it so that you only lose 1 element when you extractMax()
Hope that helps!
Upvotes: 1