Print binary tree level by level Please critique:

public void printTree(Node head) {
    if (head == null)
        return;
    Queue&lt;Node&gt; q1 = new PriorityQueue&lt;Node&gt;();
    Queue&lt;Node&gt; q2 = new PriorityQueue&lt;Node&gt;();

    q1.add(head);

    do {
        while (q1.size() &gt; 0) {
            Node element = q1.poll();
            System.out.print(element.data + " ");
            if (element.left != null)
                q2.add(element.left);
            if (element.right != null)
                q2.add(element.right);
        }
        System.out.println();
        while (q2.size() &gt; 0) {
            q1.add(q2.poll());
        }
        q2.clear();
    } while (q1.size() &gt; 0);
}
Your Node type should really be generic with Node&lt;T&gt;, so that you could easily store a Node&lt;Integer&gt;, Node&lt;Double&gt;, Node&lt;BigInteger&gt;, Node&lt;String&gt;, etc.



The function printTree accesses no external state. As such, I'd recommend it be made a static function:

public static void &lt;T&gt; printTree(Node&lt;T&gt; head) {




When I see Queue&lt;Node&gt;, I imagine a LIFO data structure. Priority queues are not LIFO, so if you really want a priority queue, I'd actually say PriorityQueue&lt;Node&gt; q1 = new PriorityQueue&lt;Node&gt;(); However, this is up to preference; in general, it is good form to use Queue&lt;Node&gt;.

On the other hand, you may be simply looking for a queue datatype. In that case, you want ArrayDeque&lt;Node&gt;.




q1.size() &gt; 0



Replace that with !q1.isEmpty(). It reads better.




while (q2.size() &gt; 0) {
    q1.add(q2.poll());
}
q2.clear();



What you are doing here is actually simply

q1.addAll(q2);
q2.clear();




Putting this all together:

public static void &lt;T&gt; printTree(Node&lt;T&gt; head) {
    if (head == null)
        return;
    Queue&lt;Node&lt;T&gt;&gt; q1 = new ArrayDeque&lt;&gt;(); // Type name can be left out here Java 7+
    Queue&lt;Node&lt;T&gt;&gt; q2 = new ArrayDeque&lt;&gt;();

    q1.add(head);

    do {
        while (!q1.isEmpty()) {
            Node&lt;T&gt; element = q1.poll();
            System.out.print(element.data + " ");

            if (element.left != null)
                q2.add(element.left);
            if (element.right != null)
                q2.add(element.right);
        }
        System.out.println();

        q1.addAll(q2);
        q2.clear();
    } while (!q1.isEmpty());
}




There are several options for improvements once you reach here. One is to create a separate Iterator&lt;Node&lt;T&gt;&gt; for your nodes, then just iterate over the iterator printing each node. But I'd also prefer this code:

public static void &lt;T&gt; printTree(Node&lt;T&gt; head) {
    if (head == null)
        return;
    Queue&lt;Node&lt;T&gt;&gt; q1 = new ArrayDeque&lt;&gt;(); // Type name can be left out here Java 7+
    Queue&lt;Node&lt;T&gt;&gt; q2 = new ArrayDeque&lt;&gt;();

    q1.add(head);

    // while loops are more common; there is no change in functionality, but may be easier to read.
    while (!q1.isEmpty()) {
        for (Node&lt;T&gt; element : q1) { // This is a for each loop
            System.out.print(element.data + " ");

            if (element.left != null)
                q2.add(element.left);
            if (element.right != null)
                q2.add(element.right);
        }
        System.out.println();

        q1 = q2; // garbage collection isn't *too* expensive, and we get more expressive this way.
        q2 = new ArrayDeque&lt;&gt;();
    }
}