CODEWITHSUNDEEP
c#.netrecursion
Iain Holder
Iain Holder

Reputation: 14282

Why isn't this recursion working in C#?

public static bool AllNodesChecked(TreeNodeCollection nodes)        
{
    foreach (TreeNode node in nodes)
    {
        if (!node.Checked)
        {
            return false;
        }
        AllNodesChecked(node.Nodes);
    }
    return true;
}

Test tree is 

A1(checked) -> B1(unchecked)
A2(checked)
A3(checked)

but it isn't returning when it hits node B1.

EDIT: Thank you all for helping my tired brain. Recursion should only be attempted early in the day after a cold shower.

Upvotes: 3

Views: 919

Answers (6)

Squirrelsama
Squirrelsama

Reputation: 5570

I have to add my two cents.... Learn to functional programming IMHO.

public static bool AllNodesChecked(TreeNodeCollection nodes)  
{
    return nodes.All(i => i.Checked && AllNodesChecked(i.Nodes));
}

Upvotes: -1

Eric Lippert
Eric Lippert

Reputation: 660088

I would take a slightly different approach here. What I'd do is I'd first write code that turns your tree (which I assume really is a tree, not an arbitrary graph) into a sequence of nodes. Something like:

static IEnumerable<Node> AllNodes(this Node node)
{
    var stack = new Stack<Node>();
    stack.Push(node);
    while(stack.Count > 0)
    {
        var current = stack.Pop();
        yield return current;
        foreach(var child in current.Nodes)
            stack.Push(child);
    }
}

and now you can use sequence operators:

bool allChecked = root.AllNodes().All(x=>x.Checked);

No recursion, no problem.

Upvotes: 7

markalex
markalex

Reputation: 126

You're not evaluating the result of the recursive call to check child nodes.

Upvotes: 3

David Gladfelter
David Gladfelter

Reputation: 4213

Try this:

public static bool AllNodesChecked(TreeNodeCollection nodes)         
{ 
    foreach (TreeNode node in nodes) 
    { 
        if (node.Checked == false || !AllNodesChecked(node.Nodes)) 
        { 
            return false; 
        } 
    } 
    return true; 
}

Upvotes: 2

Klaus Byskov Pedersen
Klaus Byskov Pedersen

Reputation: 120937

Change: 

AllNodesChecked(node.Nodes);

To:

if(!AllNodesChecked(node.Nodes))
    return false;

Upvotes: 8

Mehrdad Afshari
Mehrdad Afshari

Reputation: 421988

You are ignoring the return value of AllNodesChecked in the recursive call:

public static bool AllNodesChecked(TreeNodeCollection nodes)        
{
    foreach (TreeNode node in nodes)
        if (!node.Checked || !AllNodesChecked(node.Nodes))
           return false;
    return true;
}

The return statement only returns from the current method in the call stack to the immediate caller. It doesn't suddenly return from all other calls above in the call stack.

Upvotes: 20

Related Questions