views:

175

answers:

3

This is more of a general question so the answer does not have to be in C#. I have a general code structure below that I use to recursively walk a tree structure. In this particular case I am appending to a string builder object in each iteration.

Is there a way to improve this code to be faster or better?

    public static void RecursiveFunction(StringBuilder sb, Object treeObject)
    {
        sb.Append(treeObject.GetCurrentValue("Name"));

        if (treeObject.MoveToFirstChild())
        {
            RecursiveFunction(sb, treeObject);
        }

        if (treeObject.MoveToNextSibling())
        {
            RecursiveFunction(sb, treeObject);
        }

        if (treeObject.MoveToParent()) 
        {
            if (treeObject.MoveToNextSibling())
            {
                RecursiveFunction(sb, treeObject);
            }
        }        

        return;
    }

Thanks.

A: 
void RecurseTree(StringBuilder sb, Object tree)
{
    foreach (var child in tree.Children)
    {
        sb.Append(child.Name);
        if (child.Children.Count > 0)
        {
            RecurseTree(sb, child);
        }
    }
}
ChaosPandion
Yes, this is nice, but it uses a tree interface different from the OP's.
erikkallen
Hmmm.. looks to me like the reason he couldn't use the traditional looping constructs is that his treeObject holds a pointer to internal data structures, rather than returning them. However, to the OP, I'd recommend redesigning your treeObject to conform to the Composite pattern. Then this piece of code is just what you need.
Benjamin Cox
And? It is clear that his example was quick and dirty.
ChaosPandion
Benjamin you are very correct. In an attempt to be quick and dirty I may have confused the issue a little. I have tended to use this with objects such as the XPathNodeIterator in the past if that helps.
Ruel Waite
+2  A: 

I would probably do

public static void RecursiveFunction(StringBuilder sb, Object treeObject) {
    sb.Append(treeObject.GetCurrentValue("Name"));
    if (treeObject.MoveToFirstChild()) {
        do {
            RecursiveFunction(sb, treeObject);
        } while (treeObject.MoveToNextSiblin());
        treeObject.MoveToParent();
    }
}
erikkallen
A: 

I think this part should be redundant:

if (treeObject.MoveToParent()) 
    {
        if (treeObject.MoveToNextSibling())
        {
            RecursiveFunction(sb, treeObject);
        }
    }

The recursion should take care of that for you.

David
Removing that part ends up not traversing the entire tree. If there are nodes with multiple siblings it will still only traverse one continuous branch and return without the rest.
Ruel Waite
Looking at it again I see how you're using a pointer to move around the tree, that's not how I was thinking about it. If your function used recursion properly you wouldn't have to tell your pointer to move back up the tree. That might be part of your problem...
David