3
\$\begingroup\$

This is supposed to search for a node in a huge Treeview. The node could be found in many places. In other words there may be many nodes and the resulting tree should be expanded depending on where nodes are found.

I was asked to avoid recursivity in this method. The only solution I was able to get is using LINQ queries but I cannot since we are under 2.0 framework.

Can I avoid recursivity in this method?

  private void CheckFoundTreeNode(TreeNode startingTreeNode, List<Item> parentItems, ItemNode foundItemNode)
    {
        foreach (TreeNode childTreeNode in startingTreeNode.Nodes)
        {
            ItemNode itemNode = TreeBuilder.GetItemNode(childTreeNode);
            Item item = itemNode.Item;

            bool isParent;
            if (item.Ancestor == null)
                isParent = parentItems.Contains(item);
            else
            {
                Item ancestor = ((Grammar)item).DirectAncestor;
                isParent = parentItems.Contains(ancestor);
            }

            if (isParent)
            {
                childTreeNode.Expand();
                CheckFoundTreeNode(childTreeNode, parentItems, foundItemNode);
            }

            else if (itemNode == foundItemNode)
            {
                startingTreeNode.Expand();
                m_ListOfFoundNodes.Add(childTreeNode);
            }
        }
    }
\$\endgroup\$
8
  • \$\begingroup\$ Interesting question, I hope you get some good answers on it! \$\endgroup\$
    – Abbas
    Commented Jan 23, 2015 at 12:31
  • \$\begingroup\$ Can you add a little bit of context to your question. Like what is this supposed to do etc. \$\endgroup\$
    – Heslacher
    Commented Jan 23, 2015 at 12:37
  • \$\begingroup\$ @Heslacher well this is supposed to search for a Node on a quite huge Treeview the node coud be found on many places .. in other words there maybe many nodes and the resulted tree should be expanded depending on where nodes are found. \$\endgroup\$ Commented Jan 23, 2015 at 12:41
  • \$\begingroup\$ You should tell us a bit more about the classes involved. TreeNode seems to be a System.Web.UI.WebControls.TreeNode, but what are ItemNode and TreeBuilder, and how are they related? Where do the inputs to this function come from? \$\endgroup\$ Commented Jan 23, 2015 at 13:09
  • \$\begingroup\$ @SebastianRedl TreeNode can also come from the System.Windows.Forms namespace. But indeed, the ItemNode and TreeBuilder are a bit obscure. \$\endgroup\$
    – Abbas
    Commented Jan 23, 2015 at 13:23

2 Answers 2

4
\$\begingroup\$

The simplest way to avoid the recursive traversal is to have two lists: nodes you want to visit and nodes you have visited. Your loop then goes like this:

  1. take a node from the 'to visit' list, or end if empty
  2. check the node
  3. add any connected nodes that aren't in the 'visited' list to the 'to visit' list
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Having two list may impact performance I guess and this is exactly ,what am supposed to do, improving performance... Thank you \$\endgroup\$ Commented Jan 23, 2015 at 14:03
  • \$\begingroup\$ There are other collections that would likely be more efficient than lists. The 'to visit' collection involves adding arbitrary items to the collection and grabbing one item to process. This is what a queue is designed for. For the 'visited' collection, the only thing you care about is being able to quickly check if it has been visited. A hash-set should be good at that. But will all things performance, the only real way to know what runs fastest is to profile the implementations. \$\endgroup\$ Commented Jan 26, 2015 at 22:10
  • \$\begingroup\$ Absolutely right, I was using 'list' in a very loose way as I'm not too familiar with C# collections, but a queue and a set respectively are the right sorts of types. \$\endgroup\$
    – Danikov
    Commented Feb 1, 2015 at 1:07
1
\$\begingroup\$

This looks like a very nice implementation. An iterative solution (without linq) would only serve to obfuscate the code in my opinion. I've found that when working with trees, recursion is a much cleaner and understandable method.

At first I thought about suggesting that you change the method from void to returning a list, but I think that would be less efficient than this.

The only thing I can really note is that you should always use braces. It will prevent future bugs.

        if (item.Ancestor == null)
            isParent = parentItems.Contains(item);
        else
        {
            Item ancestor = ((Grammar)item).DirectAncestor;
            isParent = parentItems.Contains(ancestor);
        }
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.