Why heap is printing in incorrect order?

Question

I am not sure why my heap method is not printing the heap in the correct order. Can someone tell me what am I doing wrong here?

Here is my getHeap method:

    public static String getHeap(ArrayBasedList<Edge> edgeList)
{


         // Build the heap

                    insert(new Edge(-1,-1,-1));

                      for(int i = 1; i < edgeList.size(); i++)
                      {
                          // Insert into the heap
                          insert(edgeList.get(i));
                     }

                 // Print the heap
                 String output="";
                         for(int i = 1; i < edgeList.size(); i++)
                         {
                                //System.out.printf("%4d%5d\n", heap[i].getVertex1(), heap[i].getVertex2());
                                output+=String.format("%4d%5d\n", heap[i].getVertex1(), heap[i].getVertex2());
                            }

                            return output;

}

and here are my insert and upHeap methods:

    public static void insert(Edge e)
{
    heap[edgeCount] = e;
    edgeCount++;
    upHeap(edgeCount - 1);
}


public static void upHeap(int pos){
    if(pos > 0){
        if(heap[(pos-1)/2].getWeight() > heap[pos].getWeight()){
            /** Temporary edge for swapping */
            Edge temp = heap[pos];
            heap[pos] = heap[(pos-1)/2];
            heap[(pos-1)/2] = temp;
            upHeap((pos-1)/2);
        }

    }
}

and here is the print out of the edges of my heap :

but it is incorrect. the correct result is as follows:

what am I doing wrong here? would greatly appreciate any thoughts.. i am at a loss here.

sorry that was a mistake on my part. I was experimenting with code. I have now removed the ++. before it was heap[++edgeCount] = e; and now it is heap[edgeCount] = e; — learningturtle, Jul 14 '16 at 04:51
Could you please add the definition of the class `Edge` and the values that you use to create your 6 edges? From the output I can only see the values for `vertex1` and `vertex2`, but for the algorithm the `weight` is also important. — Thomas Kläger, Jul 14 '16 at 21:00

score 1 · Answer 1 · answered Jul 14 '16 at 05:02

One problem is that your upHeap method is apparently treating your heap as if the root node is at index 0, but your output code starts at index 1. So you never output the heap's root node.

Second, the array representation of a heap doesn't necessarily store things in sequential order. For example, inserting the items 1, 2, and 3 into a min-heap can create the array representation [1, 3, 2] or [1, 2, 3]. Both are valid heaps. Which one is created by inserting depends on insertion order.

If you want to remove items from the heap in order, you have to create an extractMin method, which takes the item at index 0 in the heap, and then re-adjusts the heap to move the next smallest item to index 0 and arranges the rest of the items. Typically that's done by moving the last item in the heap (i.e. the item at heap[heap.size-1] to heap[0], and then sifting it down to its proper place.

but I have no need to extractMin or deleteMin at the moment. I am only trying to print edges. — learningturtle, Jul 14 '16 at 05:31
@learningturtle: If you have no need to show them in order, then why use a heap? — Jim Mischel, Jul 14 '16 at 14:25

score 0 · Answer 2 · answered Jul 14 '16 at 04:45

0

This is probably not the whole answer, but you're increasing edgeCount twice

heap[++edgeCount] = e;
edgeCount++;

answered Jul 14 '16 at 04:45

Yaelle

401
3
5

Thomas Kläger · Answer 3 · 2016-07-14T10:37:40.547

You do not add edgeList.get(0) to your heap, so that edge (most probably the one printed out as 0 2) is missing from your output.

To add all edges to your heap your first loop should be:

for(int i = 0; i < edgeList.size(); i++)
{
    // Insert into the heap
    insert(edgeList.get(i));
}

To print out all edges your second loop needs to be:

StringBuilder output=new StringBuilder();
for(int i = 1; i <= edgeList.size(); i++)
{
    //System.out.printf("%4d%5d\n", heap[i].getVertex1(), heap[i].getVertex2());
    output.append(String.format("%4d%5d\n", heap[i].getVertex1(), heap[i].getVertex2()));
}
return output.toString();

This loop goes from 1 to edgeList.size() since your heap seems to contain a root element that should not be printed.

I've also changed to code from string concatenation to using a StringBuilder - string concatenation in a loop is slow, since strings are immutable. That might not make a big difference for small amounts of data, but it is better to always do it right.

thanks Tom. Unfortunately still incorrect output: 0 2 0 1 0 3 2 3 1 3 — learningturtle, Jul 14 '16 at 11:11

Why heap is printing in incorrect order?

3 Answers3