Home | Contact Us | FAQ | Search & Site Map | Link to Us
Sign In | Join | Other 45 Sites in Network
HomeAnnouncementsWhite Papers
Discussion GroupsFirst AidDatabasesJavaBeansGUIJava 3DVirtual MachineCORBASecurityToolsGeneral
Java DirectoryOpen Source ProjectsSample Book ChaptersUser GroupsWeb Resources
Related Topics
Databases.NETMore Topics ...

Java Forum / First Aid / February 2008

Tip: Looking for answers? Try searching our database.

PreOrder Tree Traversal

Thread view: 
Jeff Higgins - 27 Feb 2008 19:44 GMT
Need help with my Tree.PreOrderIterator,
stack is stuck;
prints ABC,
should print ABCDEFG ,
what am I doing wrong?

import java.util.ArrayDeque;
import java.util.Deque;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.NoSuchElementException;

public class Launcher {

 public static class Node {
   String data;

   public Node(String data) {
     this.data = data; }

   @Override
   public String toString() {
     return data; }
 }

 public static class Edge {
   Node source;
   Node target;

   public Edge(Node source, Node target) {
     this.source = source;
     this.target = target; }
 }

 public static class EdgeContainer {

   Edge root; Edge left; Edge right;

   public EdgeContainer() {}

   public EdgeContainer(Edge root, Edge left, Edge right) {
     this.root = root;
     this.left = left;
     this.right = right;
   }
 }

 public static class Tree {

   private final Node root;
   private final Map<Node, EdgeContainer> nodeMap;

   public Tree() {
     nodeMap = new HashMap<Node, EdgeContainer>();
     root = new Node("root");
     nodeMap.put(root, new EdgeContainer()); }

   public Node getRoot() {
     return root; }

   EdgeContainer getEdges(Node node) {
     return nodeMap.get(node); }

   public void addNode(Node node, EdgeContainer edges) {
     nodeMap.put(node, edges); }

   Iterator<Node> getPreOrderIterator(Node node) {
     return new PreOrderIterator(node); }

   private class PreOrderIterator implements Iterator<Node> {

     Deque<Node> stack = new ArrayDeque<Node>();
     boolean check = false;

     public PreOrderIterator(Node target) {
       if (target != null) {
         stack.push(target); } }

     @Override
     public boolean hasNext() {

       if (!stack.isEmpty()) {
         EdgeContainer edge = nodeMap.get(stack.pop());
         if (edge.left != null) {
           if (edge.right != null) {
             stack.push(edge.right.target); }
           stack.push(edge.left.target);
           return true; }
         else if (edge.right != null) {
           stack.push(edge.right.target);
           return true; }
       } return false; }

     @Override
     public Node next() {
       if (stack.isEmpty())
         throw new NoSuchElementException();
       return stack.peek(); }

     @Override
     public void remove() {
       throw new UnsupportedOperationException(); }
   }
 }

 public static void main(String[] args) {

   Tree tree = new Tree();

   Node na = new Node(new String("A"));
   Node nb = new Node(new String("B"));
   Node nc = new Node(new String("C"));
   Node nd = new Node(new String("D"));
   Node ne = new Node(new String("E"));
   Node nf = new Node(new String("F"));
   Node ng = new Node(new String("G"));

   /*
   root
   |
   A------F
   |      |
   |      G
   |
   B------D
   |      |
   |      E
   |
   C
    */

   EdgeContainer rootEdges =
     tree.getEdges(tree.getRoot());
   rootEdges.left = new Edge(tree.getRoot(), na);

   tree.addNode(na, new EdgeContainer(
       new Edge(tree.getRoot(), na),
       new Edge(na, nb),
       new Edge(na, nf)));
   tree.addNode(nb, new EdgeContainer(
       new Edge(na, nb),
       new Edge(nb, nc),
       new Edge(nb, nd)));
   tree.addNode(nc, new EdgeContainer(
       new Edge(nb, nc),
       null,
       null));
   tree.addNode(nd, new EdgeContainer(
       new Edge(nb, nd),
       new Edge(nd, ne),
       null));
   tree.addNode(ne, new EdgeContainer(
       new Edge(nd, ne),
       null,
       null));
   tree.addNode(nf, new EdgeContainer(
       new Edge(na, nf),
       new Edge(nf, ng),
       null));
   tree.addNode(ng, new EdgeContainer(
       new Edge(nf, ng),
       null,
       null));

   Iterator<Node> iterator =
     tree.getPreOrderIterator(tree.getRoot());

   while(iterator.hasNext()) {
     System.out.print(iterator.next().toString()); }
 }
}
Mark Space - 27 Feb 2008 20:16 GMT
> Need help with my Tree.PreOrderIterator,
> stack is stuck;

Do you have a debugger you can use?  That seems to me to be the best
idea right now...
Jeff Higgins - 27 Feb 2008 21:18 GMT
>> Need help with my Tree.PreOrderIterator,
>> stack is stuck;
>
> Do you have a debugger you can use?  That seems to me to be the best idea
> right now...

Debugger alone, no help.
Debugger with coffee, helpful;
Thanks :-)

public boolean hasNext() {
 if (!stack.isEmpty()) {
   EdgeContainer edge = nodeMap.get(stack.pop());
   if (edge.left != null) {
     if (edge.right != null) {
       stack.push(edge.right.target); }
     stack.push(edge.left.target);
     return true; }
   else if (edge.right != null) {
     stack.push(edge.right.target);
     return true; }
   else if(edge.left == null && edge.right == null) {
     if(stack.isEmpty())
       return false;
     stack.peek();
     return true; }
   else {
     return false; }
 } return false; }
Mark Space - 27 Feb 2008 22:55 GMT
> Debugger alone, no help.
> Debugger with coffee, helpful;
> Thanks :-)

Even this seems too complicated to me.  You might try to simplify your
algorithm here.  It seems to me you could do this with out any stack at
all (and a deque seems overkill to me also).

Try looking up some other tree walk algorithms, espcially those that
don't use recursion.  Morris has an algorithm, and Michael Abrash has
one too that's dead simple.

> public boolean hasNext() {
>   if (!stack.isEmpty()) {
[quoted text clipped - 15 lines]
>       return false; }
>   } return false; }
Mark Space - 27 Feb 2008 23:08 GMT
> public boolean hasNext() {

Here is Michael Abrash's algorithm, adapted to Java.  Anywhere I have
the "vistNode()" call, you should return "true."  Where I return, you
should return "false."  This is an in-order traversal, it shouldn't be
hard to adapt to pre-order.

    private void abrashInOrder() {
        // Micheal Abrash, Zen of graphics programming.
        if (this.tree == null) {
            return;
        }

        Stack<BinaryTreeNode> nodeStack = new Stack<BinaryTreeNode>();
        BinaryTreeNode curr = this.tree;

        while (true) {
            while (curr.left != null) {
                nodeStack.push(curr);
                curr = curr.left;
            }
            while (true) {
                visitNode(curr);
                if (curr.right != null) {
                    curr = curr.right;
                    break;
                }
                if (!nodeStack.isEmpty()) {
                    curr = nodeStack.pop();
                } else {
                    return;
                }
            }
        }
    }

Here is the Morris algorithm.  It uses no stack, but does modify the
tree as it traverses it.  It therefore probably is not well suited to
multi-tasking and concurrent use, but it will traverse any tree with no
additional memory requirement, making it highly deterministic, and also
very pithy on low-memory systems.

    private void morrisInOrder() {
        // J. M. Morris, "Traversing binary trees simply and cheaply"
        // Information Processing Letters, December 1979
        if (this.tree == null) {
            return;
        }
        BinaryTreeNode p = tree;
        while (p != null) {
            if (p.left == null) {
                visitNode(p);
                p = p.right;
            } else {
                BinaryTreeNode pre = p.left;
                while (pre.right != null && pre.right != p) {
                    pre = pre.right;
                }
                if (pre.right == null) {
                    pre.right = p;
                    p = p.left;
                } else {
                    pre.right = null;
                    visitNode(p);
                    p = p.right;
                }
            }
        }
    }
Jeff Higgins - 28 Feb 2008 01:03 GMT
> Jeff Higgins wrote:
>
> Even this seems too complicated to me.
> You might try to simplify your algorithm here.  It seems to me you could
> do this with out any stack at all (and a deque seems overkill to me also).

> Try looking up some other tree walk algorithms, espcially those that don't
> use recursion.  Morris has an algorithm, and Michael Abrash has one too
> that's dead simple.

Mark, thanks for the pointers.  Give me some time to
digest this, I've followed your pointers to some
online sources and need some time to sort it all out.

Is the way I'm attempting this recursive?  I guess
it is in the sense that I'm recursivly calling next()
through hasNext()?

This isn't the whole picture of what I'm attempting.
The structure I am envisioning is a binary tree,
but I think it may be a lopsided one (?).  It will
be shaped like my file system.  I also want multiple
edges and quick access to a node and its edges, hence
the Map<Node, EdgeContainer> (the first three edges
in my edge container will be used for navigation,
others may have other purposes).

I've got a working insert() method with which I can
fairly easily build my tree, I can easily find a node
and it's siblings and children, and at this moment
I'm not certain what I'm going to do with a tree
walker, but that's what I chose to go ahead and try
implement.  The next items to implement are removing
and inserting subtrees. Then ...

...

> Here is Michael Abrash's algorithm, adapted to Java. Anywhere I have the
> "vistNode()" call, you should return "true."  Where I return, you should
> return "false."  This is an in-order traversal, it shouldn't be hard to
> adapt to pre-order.

>     private void abrashInOrder() {
>         // Micheal Abrash, Zen of graphics programming.
[quoted text clipped - 58 lines]
>         }
>     }
Mark Space - 28 Feb 2008 02:30 GMT
> Mark, thanks for the pointers.  Give me some time to
> digest this, I've followed your pointers to some
> online sources and need some time to sort it all out.

If your not going through a class, try the wikipedia entries on tree
traversal.  It's a pretty basic problem in computer science and also
should be in any beginning textbook on algorithms.

> Is the way I'm attempting this recursive?  I guess
> it is in the sense that I'm recursivly calling next()
> through hasNext()?

I don't think so.  Normally recursive means the algorithm calls itself.
 next() would have to call hasNext() in turn for this to be recursive.

The classic recursive algorithm is:

  void preOrder( Tree tree )
  {
    if( tree == null )
      return;
    visit( tree );
    preOrder( tree.left );
    preOrder( tree.right );
  }

It calls itself.  You can't use recursion with iterator because you have
to return to the caller, so the iterator can't use this method of
traversal.  Recursion is also very inefficient, so it's not a great idea
in production code anyway.

"visit" a node means to do whatever work you want to do to each node.
In your case, it would just print out the data in the node.

> This isn't the whole picture of what I'm attempting.
> The structure I am envisioning is a binary tree,
> but I think it may be a lopsided one (?).  It will
> be shaped like my file system.  I also want multiple
> edges and quick access to a node and its edges, hence

I figured.  I think there may be better ways of doing this, rather than
forcing the data into a tree.  Just because it looks like a tree,
doesn't mean you should implement one.  You'll have to do some
performance testing though to determine what's best.

For example, consider just using the hash map.  Put the full string into
each node, like "\A\B\F" instead of trying to store each little string
individually, and hash on that full string.  Get the whole thing in one
look up.  Should be faster than trying to recurse a tree.
Jeff Higgins - 28 Feb 2008 21:22 GMT
>> Mark, thanks for the pointers.  Give me some time to
>> digest this, I've followed your pointers to some
>> online sources and need some time to sort it all out.

Ok, I've had time to refresh and here are my conclusions.

I think my presentation of the problem
may have been less than clear, thanks for your patience. :-)

If I get stuck, and become frustrated with a particular piece
of code; I shall not exacerbate the situation by spending even more
time and energy running for help, I'll just take a break, relax,
and have a fresher look.  I've known that for a long time but, well..

> If your not going through a class, try the wikipedia entries on tree
> traversal.  It's a pretty basic problem in computer science and also
> should be in any beginning textbook on algorithms.

Actually, that was what I had been working from.  Now your comment
reminds me of my copy of Sedgewick, Algorithms in C++, one of my
all time favorite cs books, along with Scott, Programming Language
Pragmatics, sitting on a shelf not 10 feet behind me, geesh..

>> Is the way I'm attempting this recursive?  I guess
>> it is in the sense that I'm recursivly calling next()
>> through hasNext()?

That should be "iterativly calling".

> I don't think so.  Normally recursive means the algorithm calls itself.
> next() would have to call hasNext() in turn for this to be recursive.

I didn't think so either but your comment:
"Try looking up some other tree walk algorithms, espcially those that
don't use recursion.", had me wondering for some time.  I certainly didn't
want to load up my JVM stack with a whole tree!

> The classic recursive algorithm is:
>
[quoted text clipped - 10 lines]
> to return to the caller, so the iterator can't use this method of
> traversal.

Yes, yes and yes.  This is where my presentation, and my understanding
failed.
I am(was) building an \iterator\.  The algorithm texts all present it as
a traversal \function\.  From my Sedgewick,
(I really need to purchase his new Java edition)

traverse(struct *t)
 {
   stack.push(t);
   while (!stack.empty())
     {
       t = stack.pop(); visit(t);
       if (t-.r != z) stack.push(t->r);
       if (t-.l != z) stack.push(t->l);
     }
 }

I have a \class\ PreOrderIterator implements Iterator
that I construct an instance of with a Node argument
(not necessarily the root of my tree) and then use to
iterate over, traverse, visit the nodes of, etc. my
tree from that point.

> Recursion is also very inefficient, so it's not a great idea in production
> code anyway.

If there's anyone out there that doesn't have enough to be thankful for..
Jeff Higgins is not writing code to keep heavy self-propelled vehicles
airborne, afloat, or otherwise on a straight course.
Your money and children are safe.  That's in the public domain
so no need to feel guilty about being thankful for it. :-)

> "visit" a node means to do whatever work you want to do to each node. In
> your case, it would just print out the data in the node.

I was struggling with the concepts of
visiting vs. traversing vs.iterating over.
Now I see visiting implies a pause, tea and crumpets.
When I do this:

while(iterator.hasNext()) {
     System.out.print(iterator.next().toString()); }

I am iterating over my tree, because I'm using an iterator.
My hasNext() method is providing the traversal function
and I am visiting the current node through my iterators'
next() method. (more to come on this)

>> This isn't the whole picture of what I'm attempting.
>> The structure I am envisioning is a binary tree,
[quoted text clipped - 4 lines]
> I figured.  I think there may be better ways of doing this, rather than
> forcing the data into a tree.

Now see?  I wish I hadn't done that.  The reason I interjected that
was because I had an idea that my use of an edge container had some
bearing on how the traversal algorithm worked. Silly me.

> Just because it looks like a tree, doesn't mean you should implement one.
> You'll have to do some performance testing though to determine what's
> best.

It's gonna be a tree if it wants to or not! :-)
and, I need to get it runnin' first. :-)

> For example, consider just using the hash map.  Put the full string into
> each node, like "\A\B\F" instead of trying to store each little string
> individually, and hash on that full string.  Get the whole thing in one
> look up.  Should be faster than trying to recurse a tree.

The tree is not recused! ;)

A map of paths? I expect to use some (probably smallish) maps of paths
in my grand design but not feasible for my main data model.  Also how
would I get the paths into the map witout building a tree?

Now, as promised, and if you're still with me and have interest.

From upthread you said:

"Even this seems too complicated to me.  You might try to simplify your
algorithm here.  It seems to me you could do this with out any stack at
all (and a deque seems overkill to me also)."

Are there unneccessary conditionals here,
unnecessary stack manipulations, is the
structure wrong in some way,
if not a Deque what?

 public class PreOrderIterator implements Iterator<Node> {

   Deque<Node> stack = new ArrayDeque<Node>();

   public PreOrderIterator(Node target) {
     if (target != null) { // targe's not null go ahead
       stack.push(target); } }

   @Override
   public boolean hasNext() {

     if (!stack.isEmpty()) {
       EdgeContainer edge = nodeMap.get(stack.pop());

       if (edge.left != null) {
         if (edge.right != null) {
           stack.push(edge.right.target); }
         stack.push(edge.left.target);
         return true; }

       else if (edge.right != null) {
         stack.push(edge.right.target);
         return true; }

       else if(edge.left == null && edge.right == null) {
         if(!stack.isEmpty())
           return true;
         return false; }

       else {
         return false; }

     } return false; } // stack's empty nothin left to process

   @Override
   public Node next() {
     if (stack.isEmpty())
       throw new NoSuchElementException();
     return stack.peek(); }
Mark Space - 28 Feb 2008 23:02 GMT
I think there are a couple of issue, some with my own communication at
least.  I guess I wasn't being very clear.

First, I recognize that your original program did not use recursion.
Iterators can't, I think.  So I suggested that you look up non-recursive
algorithms because that's what you have already.  No worries there.

Ok, the second thing I see is your "hasNext()" itself.  Here's
Sedgewick's algorithm:

> traverse(struct *t)
>   {
[quoted text clipped - 6 lines]
>       }
>   }

There's a strictly language issue in that I don't think pushing "null"
and then testing for empty stack is a great way to implement things in
Java, but let's let that slide.  What I see here is a way to structure
the iterator better.  Namely, looking at Sedgewick's algorithm, I see:

traverse(struct *t)                         // Constructor
  {
    stack.push(t);                          // Constructor
    while (!stack.empty())                  // Has next
      {
        t = stack.pop();                    // Has next
        visit(t);                           // Next
        if (t-.r != z) stack.push(t->r);    // Next
        if (t-.l != z) stack.push(t->l);    // Next
      }
  }

So same algorithm, just re-arrange things slightly.  Here's my result:

    class PreOrderIterator implements Iterator<BinaryTreeNode> {

        Stack<BinaryTreeNode> stack;
        BinaryTreeNode current;

        public PreOrderIterator() {
            stack = new Stack<BinaryTreeNode>();
            stack.push( tree );
        }

        @Override
        public boolean hasNext() {
            if( !stack.isEmpty() )
                current = stack.pop();
            else
                current = null;
            return current != null;
        }

        @Override
        public BinaryTreeNode next() {
            if( current.right != null )
                stack.push(current.right);
            if( current.left != null )
                stack.push( current.left );
            return current;
        }

        @Override
        public void remove() {
            throw new
UnsupportedOperationException("Not supported yet.");
        }

    }

This is a darn sight easier to understand I think, and has a lot less
branches than yours does.  It's simpler because it uses a simpler tree
node structure too, but I think the algorithm is simpler, period.

I hope you can see how I went from Sedgewick's algorithm to the iterator
one.  Try to read both a few times if you don't.  Btw, I tested the
iterator above quickly, and it seemed to work, but I can't swear there
are no bugs in it, so be careful.

This is a good skill.  You should be able to translate between
iterators, non-recursive, and recursive algorithms, just by looking at
one, and writing down the steps it uses to complete it's task.  That
proves that you really do understand what the computer is doing. When
working on a simple problem like this, you should stop and verify each
stage before going on to the next one, just so you don't chase down a
dead end due to some error in a previous step.  Sedgewick and other
references should be very handy here.

So regarding your question "what to use besides a deque?" um, a Stack
comes to mind.

Also, I think part of our miscommunication was because I was only
showing you snippets of my code.  Here's the whole thing. It's got a few
broken lines in the email due to wrapping that you'll have to fix up.
This is one file, you don't have to split anything up.

/*
 * Traverse.java
 *
 * Created on August 24, 2007, 6:34 AM
 *
 * To change this template, choose Tools | Template Manager
 * and open the template in the editor.
 */

package treetraversal;

import java.util.Iterator;
import java.util.Stack;

public class Traverse {

    BinaryTreeNode tree;
    private static final int NUM_ARRAY_SLOTS = 23;

    /** Creates a new instance of Traverse */
    public Traverse() {
    }

    /**
     * @param args the command line arguments
     */
    public static void main(String[] args) {
        // TODO code application logic here
        Traverse t = new Traverse();
        t.makeRandomTree();
        System.out.println(" == My in order traversal:");
        t.myNoRecurseInOrder();
        System.out.println("\n == Michael Abrash in order traversal:");
        t.abrashInOrder();
        System.out.println("\n == Michael Abrash in order traversal,
original algorithm:");
        t.abrashOriginalInOrder();
        System.out.println("\n == Morris in order traversal:");
        t.morrisInOrder();
        System.out.println("\n == Pre-order Iterator:");
        Traverse.PreOrderIterator iter = t.new PreOrderIterator();
        while( iter.hasNext() ) {
            t.visitNode( iter.next() );
        }
    }

    private void makeRandomTree() {
        for (int i = 0; i < 10; i++) {
            BinaryTreeNode n = new BinaryTreeNode();
            n.value = Math.random();
            insertNode(n);
        }
    }

    private void insertNode(BinaryTreeNode node) {
        if (this.tree == null) {
            this.tree = node;
        } else {
            BinaryTreeNode n = this.tree;
            while (true) {
                if (node.value < n.value) {
                    if (n.left != null) {
                        n = n.left;
                    } else {
                        n.left = node;
                        break;
                    }
                } else {
                    if (n.right != null) {
                        n = n.right;
                    } else {
                        n.right = node;
                        break;
                    }
                }
            }
        }
    }

    private void myNoRecurseInOrder() {
        // In-order tree traversal, with out using recursion
        BinaryTreeNode node = tree;
        BinaryTreeNode temp = null;
        Stack<BinaryTreeNode> nodeStack = new Stack<BinaryTreeNode>();

        // note: all of the while(true) loops are, effectively, not while
        // loops at all.  They are just place-holders for their associated
        // labels.  None of these while loops ever actually loop.
        if (tree == null) {
            return;
        }
        left_node:
        while (true) {
            while (node.left != null) {
                temp = node;
                node = node.left;
                nodeStack.push(temp);
            }

            visit_current:
            while (true) {
                visitNode(node);

                if (node.right != null) {
                    temp = node;
                    node = node.right;
                    nodeStack.push(temp);
                    continue left_node;
                } else {
                    pop_parent:
                    while (true) {
                        if (nodeStack.isEmpty()) {
                            break left_node; // DONE! Exit traversal
                        }
                        temp = nodeStack.pop();
                        if (temp.left == node) {
                            node = temp;
                            //goto visit_current
                            continue visit_current;
                        } else {
                            node = temp;
                            // goto pop_parent
                            continue pop_parent;
                        }
                    }
                }
            }
        }
    }

    private void abrashInOrder() {
        // Micheal Abrash, Zen of graphics programming.
        if (this.tree == null) {
            return;
        }

        Stack<BinaryTreeNode> nodeStack = new Stack<BinaryTreeNode>();
        BinaryTreeNode curr = this.tree;

        while (true) {
            while (curr.left != null) {
                nodeStack.push(curr);
                curr = curr.left;
            }
            while (true) {
                visitNode(curr);
                if (curr.right != null) {
                    curr = curr.right;
                    break;
                }
                if (!nodeStack.isEmpty()) {
                    curr = nodeStack.pop();
                } else {
                    return;
                }
            }
        }
    }

    private void abrashOriginalInOrder() {

        // Micheal Abrash, Zen of graphics programming.
        // Closer to original version.
        if (this.tree == null) {
            return;
        }

        BinaryTreeNode[] localStack = new BinaryTreeNode[NUM_ARRAY_SLOTS];
        BinaryTreeNode currentNode = this.tree;
        int nodeIndex = 0;
        localStack[nodeIndex++] = null;

        while (true) {
            while (currentNode.left != null) {
                localStack[nodeIndex++] = currentNode;
                currentNode = currentNode.left;
            }
            while (true) {
                visitNode(currentNode);
                if (currentNode.right != null) {
                    currentNode = currentNode.right;
                    break;
                }
                if ((currentNode = localStack[--nodeIndex]) == null) {
                    return;
                }
            }
        }
    }

    private void morrisInOrder() {
        // J. M. Morris, "Traversing binary trees simply and cheaply"
        // Information Processing Letters, December 1979
        if (this.tree == null) {
            return;
        }
        BinaryTreeNode p = tree;
        while (p != null) {
            if (p.left == null) {
                visitNode(p);
                p = p.right;
            } else {
                BinaryTreeNode pre = p.left;
                while (pre.right != null && pre.right != p) {
                    pre = pre.right;
                }
                if (pre.right == null) {
                    pre.right = p;
                    p = p.left;
                } else {
                    pre.right = null;
                    visitNode(p);
                    p = p.right;
                }
            }
        }
    }

    private void visitNode(BinaryTreeNode node) {
        System.out.println(node.value);
    }

    class PreOrderIterator implements Iterator<BinaryTreeNode> {

        Stack<BinaryTreeNode> stack;
        BinaryTreeNode current;

        public PreOrderIterator() {
            stack = new Stack<BinaryTreeNode>();
            stack.push( tree );
        }

        @Override
        public boolean hasNext() {
            if( !stack.isEmpty() )
                current = stack.pop();
            else
                current = null;
            return current != null;
        }

        @Override
        public BinaryTreeNode next() {
            if( current.right != null )
                stack.push(current.right);
            if( current.left != null )
                stack.push( current.left );
            return current;
        }

        @Override
        public void remove() {
            throw new
UnsupportedOperationException("Not supported yet.");
        }
    }

}

class BinaryTreeNode {

    public BinaryTreeNode left;
    public BinaryTreeNode right;
    public double value;
}
Jeff Higgins - 29 Feb 2008 02:24 GMT
> Jeff Higgins wrote:

I appreciate your staying with me.  Thanks.

> Ok, the second thing I see is your "hasNext()" itself.  Here's Sedgewick's
> algorithm:
[quoted text clipped - 28 lines]
>
> So same algorithm, just re-arrange things slightly.  Here's my result:

Ok, this might be source of the seeming disconnect;
May still be my thick skull however...

I'm not iterating over BinaryTreeNodes, but Nodes as defined in my OP.
I'm traversing a tree of EdgeContainers as I've defined them in my OP.
My need is to have Nodes and Edges as separate concepts, I want to
keep them and access them separately and so far this concept is working
well.
(I was going to say except for this 'traversal problem') but indeed it
is working, (at least so far as I've tested) but now my concern has become
that I implement this tree walk with unugly code.

 class Node {
   String data;
 }

 class Edge {
   Node source;
   Node target;
 }

 class EdgeContainer {
   Edge root; Edge left; Edge right;
 }

And in my Tree I have a
Map<Node, EdgeContainer> nodeMap;

I 'prime' my iterator in its constructor with a Node;
because that's what I want to have back from my next();

So, if I rewrite my iterator following the template below:

>     class PreOrderIterator implements Iterator<Node> {
>
>         Stack<EdgeContainer> stack;
>         EdgeContainer current;

         // fine

>         public PreOrderIterator(Node node) {
>             stack = new Stack<EdgeContainer>();
>             stack.push( nodeMap.get(node) );
>         }

         // fine

>         @Override
>         public boolean hasNext() {
>             if( !stack.isEmpty() )
>                 current = stack.pop();
>             else
>                 current = null;
         // Oops!
>             return current != null;
>         }

         // Now I'm going to need a BIDIMap
         // or a Map<EdgeContainer, Node>
         // in addition to my Map<Node, EdgeContainer>
         // to get back to my Node :(

>         @Override
>         public Node next() {
[quoted text clipped - 15 lines]
> This is a darn sight easier to understand I think, and has a lot less
> branches than yours does.

Yes, I agree it's easier to understand, and is certainly much prettier
but I cannot understand how to shoehorn my structure into this algorithm.

> It's simpler because it uses a simpler tree node structure too,

I guess this is the crux.

> but I think the algorithm is simpler, period.

> I hope you can see how I went from Sedgewick's algorithm to the iterator
> one.  Try to read both a few times if you don't.  Btw, I tested the
[quoted text clipped - 12 lines]
> So regarding your question "what to use besides a deque?" um, a Stack
> comes to mind.

Well, here's my inexperience showing up again.
In the Javadocs for Stack they recommend I use Deque in preference
to Stack.  Since this project is for the moment single threaded
I decided to use Deque rather than Stack and a Deque rather than
a LinkedList, because I don't need a get(int idx).

> Also, I think part of our miscommunication was because I was only showing
> you snippets of my code.

It's going to take me some time to absorb the following,
but I will follow it cause I have an interest.

Thanks
JH
Mark Space - 29 Feb 2008 05:18 GMT
>> iterator better.  Namely, looking at Sedgewick's algorithm, I see:
>>
[quoted text clipped - 11 lines]
>>
>> So same algorithm, just re-arrange things slightly.  Here's my result:

I re-wrote my version.  I think this is better for a general purpose
iterator.

    class PreOrderIterator implements Iterator<BinaryTreeNode> {

        Stack<BinaryTreeNode> stack;

        public PreOrderIterator() {
            stack = new Stack<BinaryTreeNode>();
            if( tree != null ) {
                stack.push( tree );
            }
        }

        @Override
        public boolean hasNext() {
            return !stack.isEmpty();
        }

        @Override
        public BinaryTreeNode next() {
            BinaryTreeNode current;
            if ( !stack.isEmpty() ) {
                current = stack.pop();
                if (current.right != null) {
                    stack.push(current.right);
                }
                if (current.left != null) {
                    stack.push(current.left);
                }
                return current;
            }
            else
                throw new NoSuchElementException();
        }

        @Override
        public void remove() {
            throw new
UnsupportedOperationException("Not supported yet.");
        }
    }

> Ok, this might be source of the seeming disconnect;
> May still be my thick skull however...
[quoted text clipped - 32 lines]
>>
>>         Stack<EdgeContainer> stack;
  .. get rid of the next line for the new version:
>>         // EdgeContainer current;
>
>           // fine
>
>>         public PreOrderIterator(Node node) {
>>             stack = new Stack<EdgeContainer>();
             // new code
             if( node != null )
>>             stack.push( nodeMap.get(node) );
>>         }
[quoted text clipped - 8 lines]
>>                 current = null;
>           // Oops!
            // I don't see the oops... anyway:
               return stack.isEmpty();
            // is all you need now
>>             return current != null;
>>         }
[quoted text clipped - 6 lines]
>>         @Override
>>         public Node next() {

Start at line 4 (I added line numbers above) and do the same thing:

             if( !stack.isEmpty() ) // 4
             {
               EdgeContainer current = nodeMap.get( stack.pop() );  // 6
>>             if( current.right != null )
               {
>>                 stack.push( ? );
                   stack.push( current.right.target ); // 8
               }
>>             if( current.left != null )
               {
>>                 stack.push( ? );
                   stack.push( current.right.target ); // 9
               }
>>             return ? ;
               // I'm actually not sure but maybe...

               return current.root.source;  // 7

               // or where ever you store the current node

             }
             else
                throw new NoSuchElementException();

>>         }
>>
[quoted text clipped - 5 lines]
>>
>>     }

> Well, here's my inexperience showing up again.
> In the Javadocs for Stack they recommend I use Deque in preference
> to Stack.  Since this project is for the moment single threaded

I actually missed that.  I'll check it out, thanks.


Free Magazines

Get these publications absolutely FREE for up to 12 months. There are no hidden fees and no obligation. Simply choose a title, complete the application form and submit it. Read more ...

Oracle MagazineNetwork ComputingComputer WorldBio-IT WorldeWeekInformation WeekInfosecurity
 
Sign In
Join
My Latest Posts
My Monitored Threads
My Blog
My Photo Gallery
My Profile
My Homepage

Start New Thread
Enable EMail Alerts
Rate this Thread



©2008 Advenet LLC   Privacy Policy - Terms of Use
This website includes both content owned or controlled by Advenet as well as content owned or controlled by third parties.