PriorityQueue implementation in Java with support for changePriority operation

Question

I require an implementation of a Priority queue that allows decrease priority operation to allow for an efficient implementation for Prim's and Dijkstra's algorithm.

I've coded up a minHeap implementation using a HashMap to store the indices of elements in my heap. The problem I'm working on requires the computation of the total weight of the minimum spanning tree obtained by using Prim's algorithm. While my implementation works for most test cases upto 200 nodes, I'm still getting the incorrect output for many larger test cases.

It is my understanding that such minheap based implementations of Priority queues using HashMaps are common, if I am wrong in my assumption, please provide the more suitable approach to this problem.

I've been trying to debug my code for 2 days now and it seems the only way to fix it would be to compare it with a correctly functioning implementation. Therefore, can someone please share such a PriorityQueue implementation using HashMap in java.

Even though I've tried a lot of test cases and for all the ones I can trace on my own(upto 30 nodes) I've gotten correct answers so far, but if there are some specific boundary test cases that could help me identify the problem, that too will be great.

Here is my code, I understand debugging it will be time consuming for anyone else, but if there is something obvious I've missed and someone with more expertise can point out the mistake, that would be most appreciated.

import java.util.HashMap;
import java.util.NoSuchElementException;

public class Heap<Key extends Comparable<Key>> {
    private Key[] heap;
    private int maxN, n;
    private HashMap<Key, Integer> map;
    @SuppressWarnings("unchecked")
    public Heap(int maxN) {
        if(maxN < 0 ) throw new IllegalArgumentException();
        this.maxN = maxN;
        n = 0;
        heap = (Key[]) new Comparable[maxN];
        map = new HashMap<>(maxN);
    }

    boolean isEmpty() {
        return n == 0;
    }

    boolean insert(Key e) {
        if(n +1 > maxN) throw new IllegalArgumentException("maximum capacity reached " + maxN);
        heap[n] = e;
        map.put(e,n);
        int i = n++;
        while ( (i+1)/2 - 1 >= 0){
            if ( e.compareTo(heap[(i+1)/2 - 1]) < 0 ) {
                swap(i, (i+1)/2 - 1);
                i = (i+1)/2 - 1;
            }
            else 
                break;
        }
        return true;
    }

    Key extractMin() {
        if(n == 0) throw new NoSuchElementException("Priority queue underflow ");
        Key min = heap[0];
        swap(0, n-1);
        map.remove(min);
        n--;
        int j = 0, s;
        while(j <= (n/2)-1){
            if(j == (n/2)-1 && n == (j+1)*2 )
                s = (j+1)*2 - 1;
            else 
                s = heap[(j+1)*2 - 1].compareTo(heap[(j+1)*2]) < 0 ? (j+1)*2 - 1 : (j+1)*2; 
            if(heap[j].compareTo(heap[s]) > 0 ){
                swap(j, s);
                j = s;
            }
            else break;
        }
        return min;
    }

    Key delete(Key e){
        if(!map.containsKey(e)) throw new NoSuchElementException(e+"does not exist ");
        int j = map.get(e), s;
        Key del = e;
        swap(j, n-1);
        map.remove(e);
        n--;
        while( j <= n/2 - 1){
            if(j == (n/2)-1 && n == (j+1)*2)
                s = (j+1)*2 - 1;
            else
                s = heap[(j+1)*2 - 1].compareTo(heap[(j+1)*2]) < 0 ? (j+1)*2 - 1 : (j+1)*2; 
            if(heap[j].compareTo(heap[s]) > 0 ){
                swap(j, s);
                j = s;
            }
            else break;
        }
        return del;
    }

    boolean decreasePriority(Key e){
        if(n == 0)
            return insert(e);
        if(map.containsKey(e))
            delete(e);
        return insert(e);
    }

    private void swap(int i, int j) {
        Key t = heap[i];
        heap[i] = heap[j];
        heap[j] = t;
        map.replace(heap[i], i);
        map.replace(heap[j], j);
    }

    @Override
    public String toString() {
        String res = "[";
        int i;
        for (i = 0; i < n-1; i++){
            res += heap[i] + ", ";
        }
        res += heap[i]+"]";
        return res;
    }
}

Answer 1

I think the problem is in your delete method. Your code does this:

swap item to be removed with the last item in the heap
reduce heap count
push the new item down the heap

You're making the assumption that heap[j] < heap[n-1]. That's not a valid assumption. Consider this heap:

        1
    6       2
  7   8   3

If you delete the node with value 7, the value 3 replaces it:

        1
    6       2
  3   8

You now have to move it up the tree to make a valid heap:

        1
    3       2
  6   8

The key here is that if the item you're replacing is in a different subtree than the last item in the heap, it's possible that the replacement node will be smaller than the parent of the replaced node.

If you're removing an item from the middle of the heap, you swap the item with the last, then you have to check whether the replacement node moves up or down.

Something you should consider, though, is that to change an item's priority, you don't have to delete and re-add. All you have to do is change the priority and then adjust the item's position appropriately: move up or down to put it in its new position.

Answer 2

The delete method was incorrect, I was using the same procedure for arbitrary deletes as for extractMin, which did not take into account the fact that the element I replace the key to be deleted with could possibly go both up or down the heap. Using swim() and sink() methods I have rectified this error. Also to change priority deletion and insertion are not required, and a simple call to both swim and sink is sufficient.(only swim if only decreasing priority and only sink if only increasing).

import java.util.HashMap;
import java.util.NoSuchElementException;

public class Heap<Key extends Comparable<Key>> {
    private Key[] heap;
    private int maxN, n;
    private HashMap<Key, Integer> map;
    @SuppressWarnings("unchecked")
    public Heap(int maxN) {
        if(maxN < 0 ) throw new IllegalArgumentException();
        this.maxN = maxN;
        n = 0;
        heap = (Key[]) new Comparable[maxN];
        map = new HashMap<>(maxN);
    }

    boolean isEmpty() {
        return n == 0;
    }

    boolean insert(Key e) {
        if(n +1 > maxN) throw new IllegalArgumentException("maximum capacity reached " + maxN);
        heap[n] = e;
        map.put(e,n);
        int i = n++;
        swim(i);
        return true;
    }

    Key extractMin() {
        if(n == 0) throw new NoSuchElementException("Priority queue underflow ");
        Key min = heap[0];
        swap(0, n-1);
        map.remove(min);
        n--;
        sink(0);
        return min;
    }

    void delete(Key e){
        if(!map.containsKey(e)) throw new NoSuchElementException(e+" does not exist ");
        int j = map.get(e);
        swap(j, n-1);
        map.remove(e);
        n--;
        if(!swim(j))
            sink(j);
    }

    void decreasePriority(Key e){
        if(map.containsKey(e)){
            int j = map.get(e);
            swim(j);
        }
        else insert(e);
    }

    private void swap(int i, int j) {
        Key t = heap[i];
        heap[i] = heap[j];
        heap[j] = t;
        map.replace(heap[i], i);
        map.replace(heap[j], j);
    }
    private boolean swim(int j){
        boolean change = false;
        int parent;
        while( (parent = (j-1)/2 ) >= 0){
            if(heap[j].compareTo(heap[parent]) < 0){
                swap(j,parent);
                j = parent;
                change = true;
            }
            else break;
        }
        return change;
    }
    private void sink(int j){
        while(j <= n/2 - 1){
            int leftChild = j*2 + 1, rightChild = leftChild + 1, s;
            if(rightChild >= n)
                s = leftChild;
            else
                s = heap[leftChild].compareTo(heap[rightChild]) < 0 ? leftChild : rightChild;
            if(heap[j].compareTo(heap[s]) > 0){
                swap(j,s);
                j = s;
            }
            else break;
        }
    }

    @Override
    public String toString() {
        String res = "[";
        int i;
        for (i = 0; i < n-1; i++){
            res += heap[i] + ", ";
        }
        res += heap[i]+"]";
        return res;
    }
}

Edit: Be careful with your class comparator.