0

I'm trying to remove(Comparable e) object say remove(2), but when I try to remove it removes the incorrect node in the heap and not the one that I want it to be removed.

This is what the output looks like.

The heap before removing Redwoods NP:

Bryce Canyon NP Redwoods NP Joshua Tree NP Zion NP Yosemite NP Lassen Volcanic NP 

[
null, 
Bryce Canyon NP, 
Redwoods NP, 
Joshua Tree NP, 
Zion NP, 
Yosemite NP, 
Lassen Volcanic NP
]

After removing Redwoods NP:

Bryce Canyon NP Redwoods NP Joshua Tree NP Zion NP Redwoods NP 

[
null, 
Bryce Canyon NP, 
Redwoods NP, 
Joshua Tree NP, 
Zion NP, 
Redwoods NP, 
Lassen Volcanic NP
]

BUILD SUCCESSFUL (total time: 1 second)

Expected

[
Bryce Canyon NP,
Joshua Tree NP, 
Zion NP, 
Yosemite NP, 
Lassen Volcanic NP
] 

My code

public void remove(Comparable e) throws NoSuchElementException {

    if (size == 0) {
        throw new NoSuchElementException("Heap is empty! Nothing to be removed");
    }

    Comparable toRemove = e;
    System.out.println(Arrays.toString(heapData));
    Comparable temp = heapData[size-1];
    heapData[size-1] = toRemove;
    toRemove = temp;
    size--;
    maxHeapify(heapData,size);  
}

My Add(Comparable e) code method

public void add(Comparable e) {
        if (size == heapData.length - 1) {
            doubleSize();
        }
        int position = ++size;
        for (; position > 1 && e.compareTo(heapData[position / 2]) < 0; position = position / 2) {
            heapData[position] = heapData[position / 2];
            maxHeapify(heapData, position);
        }

        heapData[position] = e;

    }
Henry
  • 13
  • 5
  • What would you expect the output to be? – Markus Deibel Mar 27 '19 at 06:33
  • The output should be, [Bryce Canyon NP,Joshua Tree NP, Zion NP, Yosemite NP, Lassen Volcanic NP] – Henry Mar 27 '19 at 06:42
  • Thank you Teasel for editing. I will do it properly next time. I am sorry for the inconvenience. – Henry Mar 27 '19 at 06:46
  • So you wanted to remove an element equal to e, what you are actually removing is the element at `heapData[size-1]` (a.k.a. the last element). I think what you wanted is `find e in heap` `swap heap_e to last` `decrease size`, you seem to be missing the `find e` loop. –  Mar 27 '19 at 12:55

1 Answers1

0

here:

toRemove = temp;

// this line is wrong, you are only changing the object in heap, not in array. instead first write a method that finds position of e in the array and assign the temp object to that position

add a method:

int findRemoveableIndex(Comparable[] input, Comparable e) {
    for(int i = 0; i < input.length; i++) {
        if(input[i].equals(e)) { // or == 
            return i;
        }
    }
    return -1;
}

then, instead of above assignment do this:

heapData[findRemoveableIndex(heapData, e)] = temp;
Ankit
  • 6,554
  • 6
  • 49
  • 71
  • Thank you Ankit. I tried this as a separate method, and I also tried a similar loop inside the remove(Comparable e) method but it seems I'm getting now, an error - java.lang.NullPointerException – Henry Mar 27 '19 at 15:00
  • @Henry share stacktrace. there is no way this code can produce NPE unless heapData is null. in that case it will crash at this line itself "Comparable temp = heapData[size-1];" – Ankit Mar 27 '19 at 15:41
  • Exception in thread "main" java.lang.NullPointerException at MaxHeap.findRemoveableIndex(MaxHeap.java:155) at MaxHeap.remove(MaxHeap.java:77) at MaxHeapTester.main(MaxHeapTester.java:19) – Henry Mar 27 '19 at 16:02
  • i see, 1st element is null. need a special handling for that at 0th index – Ankit Mar 27 '19 at 17:07
  • What should happen to the 0th index? Is my Add(Comparable e) code skipping the null element before inserting.? – Henry Mar 27 '19 at 17:27