Search code examples
javasortingpriority-queuecomparable

Java Priority Queue Not Sorting Properly


I am trying to implement a priority queue of my class type BBNode, but it doesn't seem to sift up the new nodes the way I expect it. Rather than have the smallest be at the head of the queue (how it works now), I want the largest to be there, but I can't figure out how to make this work. Here's my BBNode class.

public class BBNode implements Comparable<BBNode>{
    public int level; //level on the tree
    public int value; //value up to that node
    public int weight; //weight up to that node
    public double bound; //bound of that node

    //...constructors
    public int compareTo(BBNode node){
        if(this.bound > node.bound) return -1;
        if(this.bound < node.bound) return 1;
        return 0;
    }
}

And here's where I use the PQ.

PriorityQueue<BBNode> pq = new PriorityQueue<BBNode>();
//..other variables
while(pq.peek() != null){
  v = pq.poll();
  //System.out.println(v.weight + " " + v.value + " " + v.bound);
  if(v.bound >= bestVal && v.level < sortedItems.length-1){
     //left branch: take the next item
     u.level = v.level+1;
     u.weight = v.weight + sortedItems[u.level].weight;
     u.value = v.value + sortedItems[u.level].value;
     u.bound = bound(u);
     //System.out.println(u.bound);
     if(u.bound > bestVal){
        pq.add(u);
        System.out.println("adding " + u.bound);
        System.out.println("top of pq is " + pq.peek().bound);
     }
     if(u.weight <= maxWt && u.value > bestVal){
        bestVal = u.value;
        bestWt = u.weight;
        //System.out.println(bestVal + " " + bestWt);
        takeList[arrIdx++] = sortedItems[u.level].item+1;
     }
     //right branch: don't take the next item
     u.weight = v.weight;
     u.value = v.value;
     u.bound = bound(u);
     if(u.bound > bestVal){
        pq.add(u);
        System.out.println("adding " + u.bound);
        System.out.println("top of pq is " + pq.peek().bound);
     }
  }
}

Sorry the formatting at the end sucks. The last bracket corresponds to the while loop.

I've also tried switching around the -1 and 1 in the compare method, and I've also tried implementing a Comparator, but with the same results.

Any help is appreciated :)


Solution

  • What's likely happening is that you are modifying the bound of instances that are currently in the PriorityQueue when you do things like u.bound = bound(u);. First time through the loop, you set the u.bound and put it in, next time through you change u.bound again without pulling it off the queue first.

    If you're using an organized collection (HashSet/Map, TreeSet/Map, PriorityQueue, etc) and you change an elements value in such a way as to affect how the collection is organized, you are breaking the assumptions on which that collection is organized, and they will fail in various interesting ways. I think that's what's happening here.

    Some questions that talk about this for other collection types: