Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I use the code below for filtering a treeview. Can this be improved?

private IEnumerable<TreeNode> FindNodeByValue(TreeNodeCollection nodes, string searchstring)
 {


    foreach (TreeNode node in nodes)
    {
        if (node.Value.IndexOf(searchstring,
              StringComparison.CurrentCultureIgnoreCase) >= 0)
            yield return node;
        else
        {
            foreach (var subNode in FindNodeByValue(node.ChildNodes, searchstring))
                yield return subNode;
        }
    }
}

protected void Button1_Click(object sender, EventArgs e)
{

    var query= FindNodeByValue(TreeView1.Nodes, fieldFilterTxtBx.Text);
    if (query != null)
    {
        //TreeView1.Nodes[0].Expand();
        //TreeView1.Nodes.Clear();
        foreach (TreeNode node in query.ToList())
        {
            TreeView1.Nodes.Add(node);

        }


        //  TreeNode newnode = new TreeNode("Detail Engineering");
        // TreeView1.Nodes.Add(newnode);

        TreeView1.ExpandAll();
    }

    else
    {

        Label1.Text = "No file found";

    }
}
share|improve this question
    
What do you mean by "refresh and populate"? Do you want to clear the list and just show matching nodes? Do you want to just jump to the matching node and expand down to it? –  Bobson Nov 27 '12 at 19:50
    
i need clear the list and just show matching nodes after button click –  masoud Nov 28 '12 at 5:15

2 Answers 2

What's the problem with your implementation? You clear the tree and add new (filtered) nodes. Somehow, TreeView1.Nodes.Clear(); is under comment, so it may be supposed that you want to preserve full node structure. If so, just store it in memory:

var query = FindNodeByValue(PersistentNodeSet.Nodes, fieldFilterTxtBx.Text);
if (query != null)
{
    TreeView1.Nodes.Clear();
    foreach (TreeNode node in query.ToList())
        TreeView1.Nodes.Add(node);
    TreeView1.ExpandAll();
}

And if you want the tree to preserve original structure, you need to use recursion in FindNodeByValue().

share|improve this answer

Button1_Click

  • The var query can't be null. So replace it with if (query.Any())

  • There is no need to call ToList() you can just iterate over the items.

    foreach (TreeNode node in query)
    {
        TreeView1.Nodes.Add(node);
    
    }
    

    but basically it doesn't make much sense, because first you search in TreeView1.Nodes and then you add the found items again to the treeview.

  • If you reverse the logic of the if condition, you can save some horizontal spacing.

    if (!query.Any()) { Label1.Text = "No file found"; return; end if

    foreach (TreeNode node in query.ToList()) { TreeView1.Nodes.Add(node); }

    TreeView1.ExpandAll();

Generell

  • you should use braces {} for single if statements and loops, for your code to be less errorprone.
  • dead code should be deleted
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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