Why does PriorityQueue.toString return the wrong element order?

Question

I am trying to make a priority queue in java with the nodes with the lowest frequency in priority. However, my comparator is not working and the output is very weird. I believe I need to change my comparator but I am not sure how to change it. Here is my code:

public class HuffmanComparator implements Comparator<TreeNodeHuffman> {
    public int compare(TreeNodeHuffman p1, TreeNodeHuffman p2) {
        if (p1.frequency < p2.frequency) return -1;
        if (p1.frequency > p2.frequency) return 1;
        return 0;
    }    
}

public class TreeNodeHuffman {
public static void main(String[] args) {    
    HuffmanComparator compare = new HuffmanComparator();
    TreeNodeHuffman e = new TreeNodeHuffman('e', 12702);
    TreeNodeHuffman t = new TreeNodeHuffman('t', 9056);
    TreeNodeHuffman a = new TreeNodeHuffman('a', 8167);
    TreeNodeHuffman o = new TreeNodeHuffman('o', 7507);
    TreeNodeHuffman i = new TreeNodeHuffman('i', 6966);
    TreeNodeHuffman n = new TreeNodeHuffman('a', 6749);
    TreeNodeHuffman s = new TreeNodeHuffman('s', 6327);
    TreeNodeHuffman h = new TreeNodeHuffman('h', 6094);
    TreeNodeHuffman r = new TreeNodeHuffman('r', 5987);
    TreeNodeHuffman d = new TreeNodeHuffman('d', 4253);
    TreeNodeHuffman l = new TreeNodeHuffman('l', 4025);
    TreeNodeHuffman c = new TreeNodeHuffman('c', 2782);
    TreeNodeHuffman u = new TreeNodeHuffman('u', 2758);
    TreeNodeHuffman m = new TreeNodeHuffman('m', 2406);
    TreeNodeHuffman w = new TreeNodeHuffman('w', 2360);
    TreeNodeHuffman f = new TreeNodeHuffman('f', 2228);
    TreeNodeHuffman g = new TreeNodeHuffman('g', 2015);
    TreeNodeHuffman y = new TreeNodeHuffman('y', 1974);
    TreeNodeHuffman p = new TreeNodeHuffman('p', 1929);
    TreeNodeHuffman b = new TreeNodeHuffman('b', 1492);
    TreeNodeHuffman v = new TreeNodeHuffman('v', 978);
    TreeNodeHuffman k = new TreeNodeHuffman('k', 772);
    TreeNodeHuffman j = new TreeNodeHuffman('j', 153);
    TreeNodeHuffman x = new TreeNodeHuffman('x', 150);
    TreeNodeHuffman q = new TreeNodeHuffman('q', 95);
    TreeNodeHuffman z = new TreeNodeHuffman('z', 74);
    PriorityQueue<TreeNodeHuffman> queue = new PriorityQueue<TreeNodeHuffman>(26, compare);
    queue.add(e);
    queue.add(t);
    queue.add(a);
    queue.add(o);
    queue.add(i);
    queue.add(n);
    queue.add(s);
    queue.add(h);
    queue.add(r);
    queue.add(d);
    queue.add(l);
    queue.add(c);
    queue.add(u);
    queue.add(m);
    queue.add(w);
    queue.add(f);
    queue.add(g);
    queue.add(y);
    queue.add(p);
    queue.add(b);
    queue.add(v);
    queue.add(k);
    queue.add(j);
    queue.add(x);
    queue.add(q);
    queue.add(z);
    System.out.println(queue);
}
}

The output is as follows: [z, k, q, g, v, x, u, d, f, y, b, m, j, i, c, e, s, o, w, a, r, h, p, t, l, a].

However, the output should be [z, q, x, j, k, v, b........].

Answer 1

You need to poll the items from the PriorityQueue one by one. toString doesn't do that.

So instead of your System.out.println(queue); do this:

while(!queue.isEmpty()) {
   System.out.println(queue.poll());
}

The reason is that the PriorityQueue is never completely sorted internally, lookup how a heap works for more detail. Polling items from it fixes the heap during the calls, thus it should output the elements in sorted order.

Answer 2

The System.out.println(queue) is printing the queue unsorted. If you want to print the queue real order follow the below code which use poll to get the elements from the queue top to bottom:

TreeNodeHuffman tn = null;
    do{
        tn = queue.poll();
        if(tn!=null){
            System.out.print(tn.key+",");
        }
    }while(tn != null);

and you shall see this output as expected:

z,q,x,j,k,v,b,p,y,g,f,w,m,u,c,l,d,r,h,s,a,i,o,a,t,e,

Answer 3

You want lower frequency to go higher so :

  public int compare(TreeNodeHuffman p1, TreeNodeHuffman p2) {
          if (p1.frequency < p2.frequency) return 1;
          if (p1.frequency > p2.frequency) return -1;
          return 0;
      }    
   }

If you want to test it, send it to a single threaded pool and see the order of jobs being processed instead of to string or iterator. as doc says at http://docs.oracle.com/javase/7/docs/api/java/util/PriorityQueue.html#iterator%28%29 :

Returns an iterator over the elements in this queue. The iterator does not return the elements in any particular order.

Can see http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/Executors.html#newSingleThreadExecutor%28%29 for a quick single threaded pool to test this.

Answer 4

@Thomas answer works.

My approach is to produce same result without actually emptying the queue . So, instead I created a wrapper over the PriorityQueue, and implemented next() and hasNext() for it. Also, to exactly simulate the priority queue behavior, extend AbstractQueue and delegate calls to methods like offer, peek, poll and size through PriorityQueue object.

priorityQueueObject.methodName()

Disclaimer: This does cost copying the whole queue into a list and doing the sorting over it.

public class MyPriorityQueue<E extends Comparable<T>, T> extends AbstractQueue<E> {
    Integer arrOfInts[] = { 11, 7, 15, 10, 2, 1, 4, 5, 7, 2, 18, 1, 19};
    PriorityQueue<E> pq = new PriorityQueue<>();

    public static void main(String[] args) {
        MyPriorityQueue mpq = new MyPriorityQueue<>();
        mpq.addAll(Arrays.asList(arrOfInts));

        //Using iterator
        Iterator it = mpq.iterator();
        System.out.println("The internal priority queue:"  + mpq.pq);
        System.out.println("Using Iterator:");
        while(it.hasNext()) {
            System.out.print(it.next() + ", ");
        }

        System.out.println("\nUsing simple system out println:");
        System.out.println(mpq);

        System.out.println("Using foreach: ");
        for(Object o : mpq) {
            System.out.print(o + ", ");
        }
    }

    @Override
    public boolean offer(E arg0) {
        return pq.offer(arg0);
    }

    @Override
    public E peek() {
        return pq.peek();
    }

    @Override
    public E poll() {
        return pq.poll();
    }

    @Override
    public Iterator<E> iterator() {
        ArrayList<E> list = new ArrayList(Arrays.asList(pq.toArray()));
        Collections.sort(list, null);
        return new Iterator<E>() {
            @Override
            public boolean hasNext() {
                return !list.isEmpty();
            }

            @Override
            public E next() {
                assert (hasNext());
                return list.remove(0);
            }
        };
    }

    @Override
    public int size() {
        return pq.size();
    }
}

prints:

The internal priority queue:[1, 2, 1, 7, 5, 2, 4, 11, 7, 10, 18, 15, 19]
Using Iterator:
1, 1, 2, 2, 4, 5, 7, 7, 10, 11, 15, 18, 19, 
Using simple system out println:
[1, 1, 2, 2, 4, 5, 7, 7, 10, 11, 15, 18, 19]
Using foreach: 
1, 1, 2, 2, 4, 5, 7, 7, 10, 11, 15, 18, 19,