views:

110

answers:

3

I have the following method that works well, except the yield break statement only breaks out of the current enumerator. I understand why this is the case, but I am drawing a blank over how to propogate the yield break up through the recursive stack.

    private static IEnumerable<Node> FindChildrenById(IEnumerable nodes, string parentText) {
        var en = nodes.GetEnumerator();
        var targetFound = false;
        while (en.MoveNext())  {
            var node = en.Current as Node;
            if (node != null) 
            {
                if (node.Parent == null && string.IsNullOrEmpty(parentText))
                {
                    //Returns the top level nodes if an empty parentIdis entered
                    targetFound = true;
                    yield return node;
                }
                else if (node.Parent != null && node.Parent.Text == parentText)
                {
                    //returns the nodes belonging to the parent
                    yield return node;
                }
                else
                {
                    //Recurse into the children to see whether one of these is the node to find
                    foreach (var nd in FindChildrenById(node.Nodes, parentText))
                    {
                        yield return nd;
                    }
                }
            }
        }
        if (targetFound)
        {
            yield break;
        }
    }

So when I have the following nodes and pass "Top 2 a" as the parentText...

Top 1
    Top 1 a
    Top 1 b
Top 2
    Top 2 a
       Top 2 aa
       Top 2 ab
       Top 2 ac
    Top 2 b
Top 3
    Top 3 a
    Top 3 b
Top 4

... then I get the result:

Top 2 aa
Top 2 ab
Top 2 ac

This is the correct result, however, when I step through my code, the outer-most loop continues to process Top 3 and Top 4. How do I break out of this outer loop?

+1  A: 
//Returns the top level nodes if an empty parentIdis entered
targetFound = true;
yield return node;
yield break;

Will that work for you?

Update:

I have given it some more thought. This might be tricky with recursion. You will need to keep some state variable to break out of all the loops.

If C# had tail-recursion, I would suggest converting the code to CPS.

You could always write it in MSIL :)

leppie
@leppie I swear I see you *everywhere*, usually asking/answering questions just before I'm about to. Scary :D
Rei Miyasaka
@Rei Miyasaka: Not all the time, but I do have some bursts of `fun` :)
leppie
+1  A: 

I'm assuming that the function is actually named FindChildrenById, otherwise I can't see any recursion going on.

Under this assumption you can either use exceptions (which I would strongly recommend against), or return a KeyValuePair<bool, IEnumerable<Node>> where the bool part will be used to signal an early out up the chain.

Then, on the API level, expose a wrapper method that simply returns the IEnumerable<Node> part and throws way the bool part.

Here's an example, given the class Node:

public class Node
{
    List<Node> children;
    public string Text { get; set; }
    public List<Node> Children { get { return children ?? (children = new List<Node>()); } }
}

You can traverse and shortcut like this:

public class NodeTraverser
{
    private static KeyValuePair<bool, IEnumerable<Node>> GetChildrenById(string text, Node node)
    {
        if(node.Text == text)
        {
            return new KeyValuePair<bool,IEnumerable<Node>>(true, node.Children);
        }
        foreach(var child in node.Children)
        {
            var result = GetChildrenById(text, child);
            if(result.Key)
            {
                return result; // early out
            }
        }
        return new KeyValuePair<bool,IEnumerable<Node>>(false, Enumerable.Empty<Node>());
    }

    public static IEnumerable<Node> FindChildrenbyId(string text, Node root)
    {
        return GetChildrenById(text, root).Value; 
    }
}
corvuscorax
Yes, sorry I corrected theat little bug in my code but pasted the earlier version into the question. Thanks. I don't think I can return a KeyValuePair and still use the yield statement. It has to occur within a method that returns IEnumerable<T>
Daniel Dyson
I added an example of what I mean .. the yield statement isn't strictly necessary, is it?
corvuscorax
Yes, in this case. I am dealing with a huge number of nodes which are converted and manipulated. By using yield return and yield break I am able to break out of the loop without converting or manipulating the remainder once I have found what I am looking for. Your example requires that the entire collection is preloaded, converted and manipulated before any processing occurs. But thanks anyway.
Daniel Dyson
Sorry, but why do you think it requires that everything is preloaded?
corvuscorax
The exception idea is not so bad, it will only be thrown once you find a value, so performance wont suffer too much. This is a also a commonly used pattern for non-local returns in functional languages. See http://en.wikipedia.org/wiki/Continuation.
leppie
The reason why I don't like using exceptions for this, is not so much because of performance considerations, but because I consider it poor design to use exceptions for signalling normal conditions. It also doesn't play nicely with logging and other dependency injections that assume certain reasonable patterns over the entire code base.
corvuscorax
+1  A: 

If I got your code wirte I guess the below code will solve youre problem

private static IEnumerable<Node> FindChildrenById(IEnumerable nodes, string parentText)
    {
        var result =
               (from node in nodes
                where (node.Parent == null && string.IsNullOrEmpty(parentText))
                      || (node.Parent != null && node.Parent.Text == parentText)
                select node).TakeWhile(node => !(node.Parent == null && string.IsNullOrEmpty(parentText)));
        return result;
    }

it's build on two extension methods (see below) and should only iterate until your target found criteria is met

public static class IEnumerablExtensions
        {
            //Will iterate the graph in depth first order
            public static IEnumerable<TResult> Select<TResult>(this IEnumerable collection, Func<Node, TResult> selector)
            {
                foreach (var obj in collection)
                {
                    var node = obj as Node;
                    if (node != null)
                    {
                        yield return selector(node);
                        foreach (var n in node.Nodes.Select(selector))
                        {
                           yield return n;
                        }
                    }
                }
            }

        public static IEnumerable<Node> Where(this IEnumerable collection, Predicate<Node> pred)
        {
            foreach (var node in collection.Select(x => x)) //iterate the list in graph first order
            {
                if (pred(node))
                    yield return node;
            }
        }
    }

EDIT: There was an error in the Selct method in the original posting (it didn't iterate the children of children) that is now corrected

Rune FS
Thanks @Rune FS. I will try this and report back.
Daniel Dyson