1

I was trying to solve a problem:

Print all paths of a binary tree, from root to leaf.

I wrote the following code, the result is correct:

public void printPath(TreeNode root) {

    printDFS(root, new int[1000], 0);

}



private void printDFS(TreeNode r, int[] path, int idx){
    if (r == null) return;

    path[idx] = r.val;
    idx++;

    /* if it's leaf, print path: from root to leaf */
    if (r.left == null && r.right == null) 
        printOnePath(path, idx);

    else {
        /* go left, go right */
        printDFS(r.left,  path, idx);
        printDFS(r.right, path, idx);
    }
}


private void printOnePath(int[] path, int idx) {
    for (int i = 0; i < idx; i++) {
        System.out.print(path[i] + " ");
    }
    System.out.println();         
}

However,
when I tried to use ArrayList to store the path data, instead of int[].
This method becomes wrong.
And the output results really lost me.

public void printPath(TreeNode root) {

    printDFS(root, new ArrayList<Integer>(), 0);        

}

private void printDFS(TreeNode r, List<Integer> path, int idx){
    if (r == null) return;

    path.add( r.val );
    idx++;

    /* if it's leaf, print path: from root to leaf */
    if (r.left == null && r.right == null) 
        printOnePath(path, idx);

    else {
        /* otherwise: go left, go right */
        printDFS(r.left,  path, idx);
        printDFS(r.right, path, idx);
    }
}


private void printOnePath(List<Integer> path, int idx) {
    for (int i = 0; i < idx; i++) {
        System.out.print(path.get(i) + " ");
    }
    System.out.println();         
}

E.g:

For a binary tree:
enter image description here

First STDOUT is: (Correct) 10 5 3 3 10 5 3 -2 10 5 2 1 10 -3 11

Second STDOUT is: (Wrong) 10 5 3 3 10 5 3 3 10 5 3 3 10 5 3

I believe the initial ArrayList has already set as empty at the very beginning of each DFS. Why is the result totally different with an int array, even using the same method?

Does anyone know why? Thanks a lot!

Peter
  • 155
  • 2
  • 10

3 Answers3

2

The difference is that idx, an int, is passed by value. However, the ArrayList path is an object; thus, while the parameter, which is a reference, is passed by value, the object that path refers to is not passed by value and is shared among all the invocations of your recursive method.

This means that in your first method, when you call this recursively:

    printDFS(r.left,  path, idx);
    printDFS(r.right, path, idx);

Suppose idx is 3 when you call printDFS on r.left. When the recursive method increments idx with idx++, they are incrementing a new copy of idx. Thus, when the recursive methods finally wind down and return, the copy of idx that this method is using is still 3, so it will be calling printDFS(r.right, path, 3). This makes a difference because you use idx to set path[idx].

In your second method, idx behaves the same way, but you're not using it when you build the ArrayList. Instead, you add to the ArrayList. So when you call

    printDFS(r.left,  path, idx);
    printDFS(r.right, path, idx);

Now, if path has 3 elements when you call printDFS on r.left, when the recursive calls wind down and return, they have added to the ArrayList--and since this object is shared among all recursive invocations, path will still contain all the elements that the recursive invocations added to it. So unlike the first code example, you are not calling printDFS(r.right...) with the same values as you called printDFS(r.left...).

There are several ways to solve this:

  1. Use idx to set the array elements, as another answer suggested. You'll need to check whether idx is the index of an existing element, though, and use set if the element exists or add if idx equals path.size().

  2. Make a copy of path before calling each printDFS. This ensures that all recursive invocations have their own clean copy of the array. (Or make one copy at the beginning of printDFS and work with that instead of the original path; this should ensure that no printDFS modifies its caller's path.)

  3. Make sure each printDFS leaves the path the way it found it. Thus, since printDFS adds a new element to the end of the array, make it delete the last element of the array before it returns. (I believe this will work, although I haven't tested it. However, I don't recommend this approach in general; with more complex recursive situations, getting the "cleanup" right can be very difficult.)

ajb
  • 31,309
  • 3
  • 58
  • 84
2

I followed copy strategy as ajb mentioned(option #2 in this comment). Here is the modified code.

    import apple.laf.JRSUIUtils;

    import java.util.ArrayList;
    import java.util.List;

    /**
     * Created by anilwaddi on 8/1/17.
     */
    public class DFSTest {


      public static void main(String args[]) throws Exception {

        DFSTest test = new DFSTest();
        test.printDFS();
    /*
    10 5 3 3
    10 5 3 -2
    10 5 2 1
    10 -3 11
     */
      }

      TreeNode root;

      DFSTest() {
        TreeNode node41 = new TreeNode(3, null, null);
        TreeNode node42 = new TreeNode(-2, null, null);
        TreeNode node43 = new TreeNode(1, null, null);


        TreeNode node31 = new TreeNode(3, node41, node42);
        TreeNode node32 = new TreeNode(2, node43, null);
        TreeNode node33 = new TreeNode(11, null, null);

        TreeNode node21 = new TreeNode(5, node31, node32);
        TreeNode node22 = new TreeNode(-3, node33, null);
        root = new TreeNode(10, node21, node22);
      }

      public void printDFS() {
        printPath(root);
      }

      public void printPath(TreeNode root) {
        printDFS(root, new ArrayList<Integer>());

      }

      private void printDFS(TreeNode r, List<Integer> path ) {
        if (r == null) return;

        path.add(r.val);

        /* if it's leaf, print path: from root to leaf */
        if (r.left == null && r.right == null)
          printOnePath(path );

        else {
            /* otherwise: go left, go right */
          List<Integer> newPathLeft = new ArrayList<>();
          newPathLeft.addAll(path);
          printDFS(r.left, newPathLeft);

          List<Integer> newPathRight = new ArrayList<>();
          newPathRight.addAll(path);
          printDFS(r.right, newPathRight);
        }
      }


      private void printOnePath(List<Integer> path ) {
        for (int i = 0; i < path.size(); i++) {
          System.out.print(path.get(i) + " ");
        }
        System.out.println();
      }

      private class TreeNode {
        TreeNode left;
        TreeNode right;
        Integer val;

        TreeNode(Integer val) {
          this.val = val;
        }

        TreeNode(Integer val, TreeNode left, TreeNode right) {
          this.val = val;
          this.left = left;
          this.right = right;
        }
      }
    }
Anil
  • 595
  • 1
  • 5
  • 15
-1

The lines:

path[idx] = r.val;

and

path.add(r.val);

are not equivalent.

The result is that subsequent paths, after the first one, are getting added to the end of the list. When you print the path these aren't being seen, since you're only printing up to idx elements.

I don't believe there is a direct List equivalent of array assignment in Java.

Calling

path.set(idx, r.val);

doesn't work since Java doesn't grow Lists automatically.

Calling

path.add(idx, r.val);

doesn't really work either as it shifts existing elements to the right.

If you want to use a List I think you're going to have to clear it after printing it.

RaffleBuffle
  • 5,396
  • 1
  • 9
  • 16
  • `path.set(idx, r.val)` is not equivalent to `path[idx] = r.val`, because `set` only works if the index already exists in `path`. You can never use `set` to add a new value to the `ArrayList` and make it longer. – ajb Aug 02 '17 at 03:52
  • Still no good. Using the two-argument `add` doesn't work like `set`; it will insert the element. The only way to get this right is with an `if` that either uses `add` to the end of the list, or `set`, depending on whether `idx` equals the list size. – ajb Aug 02 '17 at 04:06