HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Recursive Level Order traversal

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
levelrecursiveordertraversal

Problem

I have come up with this code to do level order traversal recursively for a binary search tree.

private void printLevelOrder_Recursive(Node root2) {
    Queue queue = new LinkedList();
    queue.add(root2);

    Stack stack = new Stack();
    stack.add(root2);
    if(stack.isEmpty() == false) {

        Node node = stack.pop();
        System.out.println(node.data);

        stack.addAll(printLevelOrder_Recursive_Children(node.left, node.right));
    }

    while(stack.isEmpty() == false) {
        Node node = stack.pop();
        System.out.println(node.data);
    }
}

private Stack printLevelOrder_Recursive_Children(Node left, Node right) {
    Stack stack = new Stack();
    if (left == null && right == null) {
        return stack;
    }

    Stack leftChildren = new Stack();
    Stack rightChildren = new Stack();
    if (left != null && (left.left != null || left.right != null)) {
        leftChildren = printLevelOrder_Recursive_Children(left.left, left.right);
    }
    if (right != null && (right.left != null || right.right != null)) {
        rightChildren = printLevelOrder_Recursive_Children(right.left, right.right);
    }

    if (rightChildren.isEmpty() == false) {
        stack.addAll(rightChildren);
    }
    if (leftChildren.isEmpty() == false) {
        stack.addAll(leftChildren);
    }
    if (right != null) {
        stack.add(right);
    }
    if (left != null) {
        stack.add(left);
    }
    return stack;
}


where Node is:

public class Node {
    public int data;
    public Node left;
    public Node right; 
}


I looked at various implementations, but I find mine much easier. Could you please chime in with your review and thoughts?

Solution

A few things that catch my eye, just skimming over your code:

-
Unconventional method names:

Java method names are supposed to be named strictly in camelCase. You're using some bastardized camel-snake-pascal-case which is something I'd expect when mixing together PHP, Python and C++... ~shivers

-
Use of outdated API:

Stack is a class in Java retroactively added to the Collections API, but it should've been officially deprecated since Java 1.5. It's been officially replaced by Deque, which is an interface that supports all stack-operations, see also Stack's javadoc:


A more complete and consistent set of LIFO stack operations is
provided by the Deque interface and its implementations

-
Returning a Collection from a method which begins with print

A method name should not lie. For a method named print... I'd expect to give something to be printed and some output channel (e.g. BufferedOutputWriter) with no return value. Instead you should name it something like traverseLevelOrder.

-
Overcomplicated helper method:

For a recursive traversal it's definitely not necessary to have a method that takes two nodes. Instead of relying on the caller to maintain the Stack you have, you may want to give the Stack (or better the Deque) into the recursive traversal for a level-order traversal. Also it's a significant difference whether you only want to traverse the nodes or also keep the traversal results in memory...

-
General recursive approach:

It might be a better option to actually do the level-order traversal with an "iterative" approach, by maintaining the nodes you still have to traverse in a queue that builds up by level and just process them back again. Following code should give you an idea:

void traverse(Node node) {
    Queue traversalQueue = new LinkedList<>();
    traversalQueue.add(node);
    while (!traversalQueue.isEmpty()) {
        // add nodes to queue by level
        // and pop the current:
        traversalQueue.add(node.left); // null-checks omitted
        traversalQueue.add(node.right);
        // use node.data
    }
}

Code Snippets

void traverse(Node node) {
    Queue<Node> traversalQueue = new LinkedList<>();
    traversalQueue.add(node);
    while (!traversalQueue.isEmpty()) {
        // add nodes to queue by level
        // and pop the current:
        traversalQueue.add(node.left); // null-checks omitted
        traversalQueue.add(node.right);
        // use node.data
    }
}

Context

StackExchange Code Review Q#92899, answer score: 4

Revisions (0)

No revisions yet.