2
\$\begingroup\$

I want you to pick my code apart and give me some feedback on how I could make it better or more simple. This code will traverse peripheri (clock and anticlockwise) in \$O(n)\$ just traversing the tree once.

public class PeriPheri {

    private TreeNode root;

    private static class TreeNode {
        TreeNode left;
        TreeNode right;
        int item;

        TreeNode(TreeNode left, TreeNode right, int item) {
            this.left = left;
            this.right = right;
            this.item = item;
        }
    }

    private void printChild (TreeNode node) {
        if (node == null) {
            return;
        }

        if (node.left == null && node.right == null) {
            System.out.print(node.item + " ");
        }

        if (node != null) {
            printChild (node.left);
            printChild (node.right);
        }
    }

    private void leftAntiClock(TreeNode node) {
        if (node != null) {
            System.out.print(node.item +  " ");
            leftAntiClock (node.left);
            printChild (node.right);
        }
    }

    private void rightAntiClock(TreeNode node) {
        if (node != null) {
            printChild (node.left);
            rightAntiClock (node.right);
            System.out.print(node.item + " ");
        }
    }

    public void antiClockwise() {
        if (root == null) {
            return;
        }

        System.out.print (root.item + " ");
        leftAntiClock (root.left);
        rightAntiClock (root.right);
    }


    private void rightClock(TreeNode node) {
        if (node != null) {
            System.out.print(node.item + " ");
            rightClock(node.right);
            printChild(node.left);
        }
    }

    private void leftClock(TreeNode node) {
        if (node != null) {
            printChild(node.right);
            printChild(node.left);
            System.out.print(node.item + " ");
        }
    }

    public void clockwise() {
       if (root == null) {
          return null;
        }
        System.out.print(root.item + " ");
        rightClock (root.right);
        leftClock (root.left);
    }

}
\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

The way you handle NULL in private void printChild (TreeNode node) { is confusing and potentially wrong. It would be nice if it could be handled in a consistent way across the different methods : either if (n==null) return; or if (n!=null) { your_code }. That would make everything easier to follow and that would make duplicated code easier to spot

I'd change the order of the argument in the constructor and define : TreeNode(int item, TreeNode left, TreeNode right) AND TreeNode(int item) (initialising trees to NULL)

\$\endgroup\$
1
\$\begingroup\$

I think that printChild() would be better named printDescendantLeaves(). Also, printChild() always works left-to-right, which spoils your clockwise traversal.

For flexibility, consider taking a visitor instead of hard-coding System.out.println() everywhere. Alternatively, implement your traversal as an Iterator (which is admittedly tricky to do recursively since Java doesn't support yield).

"Periphery" is misspelled.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks for feedback, I do appreciate feedback on visitor pattern, but I am preparing strictly for interviews and doubt if visitor would be necessary on white board. \$\endgroup\$ Sep 28, 2013 at 18:18
  • 1
    \$\begingroup\$ Demonstrating knowledge of the visitor pattern in an interview bumps you up to the next pay grade. =) \$\endgroup\$ Sep 28, 2013 at 19:27
  • \$\begingroup\$ good point sir. accepted. \$\endgroup\$ Sep 28, 2013 at 20:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.