1

so I'm working on a lab for my class. I need to create a Priority Queue in Java using an array list. The kicker is that each node has to have a "handle" which is just an object which contains the index of of the node that it's associated with. I know that sounds weird but we have to implement it this way. Anyway, my priority queue seem to work fine except the handle values apparently aren't updating correctly because I fail the handle test. Also, I run into problems with handles only after extractMin() is called, so I figured the problem would be somewhere in that method but I've been through it a million times and it looks good to me.

Here is my Priority Queue Class:

package lab3;

import java.util.ArrayList;

/**
 * A priority queue class supporting operations needed for
 * Dijkstra's algorithm.
 */

class PriorityQueue<T> {

final static int INF = 1000000000;
ArrayList<PQNode<T>> queue;

/**
 * Constructor
 */
public PriorityQueue() {
    queue = new ArrayList<PQNode<T>>();
}

/**
 * @return true iff the queue is empty.
 */
public boolean isEmpty() {
    if (queue.size() <= 0)
        return true;
    return false;
}

/**
 * Insert a (key, value) pair into the queue.
 *
 * @return a Handle to this pair so that we can find it later.
 */
Handle insert(int key, T value) {
    queue.add(new PQNode(key, value));    

    int loc = queue.size()-1;
    // Swap with parent until parent not larger
    while (loc > 0 && queue.get(loc).getKey() < queue.get(parent(loc)).getKey()) {
        swap(loc, parent(loc));
        loc = parent(loc);
    }
    return new Handle(loc);
}

private void swap(int i, int j) {
    PQNode t = queue.get(i);

    queue.set(i, queue.get(j));
    queue.get(i).setHandleIndex(i);

    queue.set(j, t);
    queue.get(j).setHandleIndex(j);
}


/**
 * @return the min key in the queue.
 */
public int min() {
    if (isEmpty())
        return -INF;
    return queue.get(0).getKey();
}

/**
 * Find and remove the smallest (key, value) pair in the queue.
 *
 * @return the value associated with the smallest key
 */

public T extractMin() {
    if (isEmpty())
        return null;
    else{
        T value = queue.get(0).getValue();
        swap(0, queue.size()-1);
        queue.get(queue.size()-1).setHandleIndex(-1);
        queue.remove(queue.size()-1);
        heapify(0);
        return value;
    }
}



private void heapify(int i) {
    int left = leftChild(i);
    int right = rightChild(i);
    int min;

    if (left <= queue.size()-1 && queue.get(left).getKey() < queue.get(i).getKey())
        min = left;
    else
        min = i;
    if (right <= queue.size()-1 && queue.get(right).getKey() < queue.get(min).getKey())
        min = right;
    if (min != i){
        swap(i, min);
        heapify(min);
    }
}
private static int parent(int i) {
    return (i-1)/2;
}

private static int leftChild(int i) {
    return 2*i + 1;
}

   /**
 * @return the key associated with the Handle.
 */
public int handleGetKey(Handle h) {
    if (h.getIndex() >= 0)
        return queue.get(h.getIndex()).getKey();
    return -INF;
}


/**
 * Decrease the key to the newKey associated with the Handle.
 *
 * If the pair is no longer in the queue, or if its key <= newKey,
 * do nothing and return false.  Otherwise, replace the key with newkey,
 * fix the ordering of the queue, and return true.
 *
 * @return true if we decreased the key, false otherwise.
 */
public boolean decreaseKey(Handle h, int newKey){
    if (h.getIndex() == -1) //negative 1 is my signal for an invalidated handle
        return false;
    if (newKey > queue.get(h.getIndex()).getKey())
        return false;
    queue.get(h.getIndex()).setKey(newKey); // set the new key
    correct(h.index);       // restore the min-heap property
    return true;
}
public void correct(int i)
{
    {
        while (i > 0 && queue.get(parent(i)).getKey() > queue.get(i).getKey()) {
            swap(i, parent(i));
            i = parent(i);
        }
    }   
}



/**
 * Return the index of the right child of node i.
 * @param i index of parent
 * @return the index of the right child of node i
 */
private static int rightChild(int i) {
    return 2*i + 2;
}

/**
     * @return the value associated with the Handle.
     */
public T handleGetValue(Handle h) {
        if (h.getIndex() >= 0){
            return queue.get(h.getIndex()).getValue();}
        else{
            return null;
            }
    }

And my PQNode class:

package lab3;

public class PQNode<T> {

int key;
T value;
Handle handle;

PQNode(int k, T v){
    key = k;
    value = v;
    handle = new Handle(0);
}

/**
 * @return the key
 */
public int getKey() {
    return key;
}

/**
 * @param key the key to set
 */
public void setKey(int key) {
    this.key = key;
}

/**
 * @return the value
 */
public T getValue() {
    return value;
}

/**
 * @param value the value to set
 */
public void setValue(T value) {
    this.value = value;
}

public void setHandle(Handle handle){
    this.handle = handle;
}

/**
 * @return the handle
 */
public Handle getHandle() {
    return handle;
}
public int getHandleIndex(){
    return handle.getIndex();
}
public void setHandleIndex(int i){
    handle.setIndex(i);
}
}

And my Handle Class:

package lab3;

public class Handle {

int index;


public Handle(int i) {
    index = i;
}

/**
 * @return the index
 */
public int getIndex() {
    return index;
}

/**
 * @param index the index to set
 */
public void setIndex(int index) {
    this.index = index;
}
}

Because this is school work I'd appreciate hints, but try not to just blurt out the answer. Also, I can post the tests too if you'd like but I don't know if that would do much good. Just know that the queue works as is should, but the handles don't point to the right nodes after extractMin() is utilized. Please tell me if you'd like more information, I know this is all a little vague. Thanks so much guys.

EDIT: Here is the specefic test I am failing

 public void basicHandleAndDecreaseTest() {
        PriorityQueue<String> q = new PriorityQueue<String>();
        Map<String, Handle> valuesToHandles = new HashMap<String, Handle>();
        Map<String, Integer> valuesToKeys = new HashMap<String, Integer>();
        int seq = 0;
        // Generate random numbres for the keys.
        for (int i = 0; i < 20; i++) {
            int key = RAND.nextInt(100);
            String val = Integer.toString(seq); // guarantees that the value is unique.
            seq++;
            valuesToKeys.put(val, key);
        }
        // Keep track of handles and build queue.
        for (Map.Entry<String, Integer> entry : valuesToKeys.entrySet()) {
            int key = entry.getValue();
            String value = entry.getKey();
            Handle h = q.insert(key, value);
            assertEquals(key, q.handleGetKey(h));
            assertEquals(value, q.handleGetValue(h));

            valuesToHandles.put(value, h);
        }

        // Remove some of the elements.
        for (int i = 0; i < 5; i++) {
            String val = q.extractMin();
            valuesToKeys.remove(val);
            valuesToHandles.remove(val);
        }
        // Make sure the handles are still valid.
        for (Map.Entry<String, Integer> entry : valuesToKeys.entrySet()) {
            int key = entry.getValue();
            String value = entry.getKey();
            Handle h = valuesToHandles.get(value);
            assertEquals(value, q.handleGetValue(h));  <--Here is when I fail

        }

        // Add some more elements.
        int limit = seq + 5;
        for (int i = seq; i < limit; i++) {
            int key = RAND.nextInt(100);
            String val = Integer.toString(seq);
            seq++;
            valuesToKeys.put(val, key);
            Handle h = q.insert(key, val);
            valuesToHandles.put(val, h);
        }

        // Decrease keys unnecessarily and verify that they still work.
        for (Map.Entry<String, Integer> entry : valuesToKeys.entrySet()) {
            int key = entry.getValue();
            String value = entry.getKey();
            Handle h = valuesToHandles.get(value);
            q.decreaseKey(h, key + 1);
            assertEquals(key, q.handleGetKey(h));
            assertEquals(value, q.handleGetValue(h));
        }

        // Change keys randomly, and check that the values remain attached to handles.
        for (Map.Entry<String, Integer> entry : valuesToKeys.entrySet()) {
            int key = entry.getValue();
            String value = entry.getKey();
            Handle h = valuesToHandles.get(value);
            int newKey = RAND.nextInt(100);
            q.decreaseKey(h, newKey);
            if (newKey < key) {
                assertEquals(newKey, q.handleGetKey(h));
            } else {
                assertEquals(key, q.handleGetKey(h));
            }
            assertEquals(value, q.handleGetValue(h));
        }
    }
  • How aren't the handle values updating correctly? Do they have nonsensical values, off-by-one values, don't change at all, or something else? – awksp May 05 '14 at 01:02
  • +1 for a well-asked homework question, sadly all too rare here. – user207421 May 05 '14 at 01:12
  • The handles seem to always be less that what they're supposed to be, but the test generate a bunch of random nodes, inserts them, and, extracts a few, than rechecks the handles to see if they are still valid, and that's where I fail. The amount by which they differ seems random though –  May 05 '14 at 01:19
  • 1
    Why would the handles be the same? You've moved things around in the heap. Maybe you aren't supposed to update the handles when you do that? And how can the test 'recheck the handles'? – user207421 May 05 '14 at 01:23
  • No I have to update the handles. And the handles aren't supposed to stay the same. They are supposed to change, and the tests know how they are supposed to change. –  May 05 '14 at 01:35
  • I suggest you post the test that's failing. – user207421 May 05 '14 at 01:38
  • Alright I added the test, thanks for your help so far, I also marked that place where it fails –  May 05 '14 at 01:52
  • Where are your implementations of handleGetKey() and decreaseKey()? – user207421 May 05 '14 at 02:41

1 Answers1

0

Without blurting out the answer, the handle you return when inserting should be the handle of the node you just inserted, not a new one. That way it will get updated when you do the swaps. The node's handle also needs to be initialized to something more relevant than zero.

That will get you to the next error ... which is either:

  1. A bug in the test program, in the block labelled 'Decrease keys unnecessarily and verify that they still work.'. The wrong 'key' value is being tested. The key in the node has been changed to key+1, so that is what should be tested, not key. The same applies to the block labelled 'Change keys randomly, and check that the values remain attached to handles', only this time the new key is random, not key+1.

    or

  2. that the meaning of decreaseKey() eludes me.

user207421
  • 305,947
  • 44
  • 307
  • 483
  • Okay so I replaced my return statement with this: return queue.get(loc).getHandle(); for the handle intialization then would I set it the the last index of the queue perhaps? Also thank you so much –  May 05 '14 at 02:55
  • Oh actually I think that did, now I just need to implement decreaseKey correctly, but you're solution fixed the issue. Thank you so much. I really appreciate it. –  May 05 '14 at 03:10
  • What is `decreaseKey()` supposed to do? – user207421 May 05 '14 at 03:16
  • It's supposed to just update the key of any given node to a specified key, which I'm assuming is supposed to be lower than the current key. Let me add the method to the code along with the method header which has the details. I forgot to include it when I posted the question. –  May 05 '14 at 03:27
  • 1
    OK your `decreaseKey()` method needs work. The test program is OK. You didn't post the `contains()` or `correct()` methods but the line containing the `contains()` call does not correctly implement the constraint expressed in the comment above. The `correct()` method seems to be just another name for `heapify().` – user207421 May 05 '14 at 03:43
  • Alright I made some changes to my decreaseKey() and I will add my correct method to the code, just give me a second to edit my post. I see the problem with my contains method, so I did away with it all together and just checked the validity of the handle (see if it was equal to -1) –  May 05 '14 at 04:16
  • Oh wow! The code works now. Awesome I think I'm good to go. You have no idea how greatful I am for your help. I'm also very impressed with your skill, you have true talent in programming. –  May 05 '14 at 04:22