4
\$\begingroup\$

I am using the following code to add nodes in a treeview. Is there a better or more compact way of doing this by using LINQ?

foreach (Plan plan in this.IncomingPlan.Plans)
{
    foreach (Document doc in plan.Documents.Where(d => d.Name.Equals(this.DocumentName, StringComparison.OrdinalIgnoreCase)))
    {
        foreach (Author author in doc.Authors)
        {
            TreeNode treeNode = new TreeNode()
            {
                Text = author.Name,
                Type = NodeType.ParentNode,
                Tag = author
            };

            foreach (Book book in author.Books)
            {
                treeNode.Nodes.Add(new TreeNode()
                {                    
                    Text = book.Name,
                    Type = NodeType.ChildNode,
                    Tag = book
                });
            }

            this.treeView.Nodes.Add(treeNode);
        }
    }
}
\$\endgroup\$
2
\$\begingroup\$

You can make this more maintainable and more compact by utilizing LINQ here. I'm not sure what TreeNode is in your code, I'm guessing you derived from a WinForms TreeNode.

I'd argue that the node's Type is unnecessary. You can easily determine that if you look at its Level. Level 0 indicates it is at the root of the tree, otherwise it is greater than 0.

Unfortunately there's no nice way to add a range of nodes to another. You could only add arrays of the nodes. Using a loop would be the best option.

Here, I would flatten the nested loops as far as I can then loop through to add them to the tree. To compact it even more, create a factory method to create the nodes. Even more useful if you have a lot of properties to set.

// create the node for the item
static TreeNode CreateNode<T>(T item, Func<T, string> textSelector)
{
    return new TreeNode { Text = textSelector(item), Tag = item };
}

var authors =
    from plan in this.IncomingPlan.Plans
    from doc in plan.Documents
    where doc.Name.Equals(this.DocumentName, StringComparison.OrdinalIgnoreCase)
    from author in doc.Authors
    select author;

foreach (var author in authors)
{
    var authorNode = CreateNode(author, a => a.Name);

    foreach (var book in author.Books)
    {
        authorNode.Nodes.Add(CreateNode(book, b => b.Name));
    }

    treeView.Nodes.Add(authorNode);
}
\$\endgroup\$
  • \$\begingroup\$ Might be handy to write an extension method on TreeNodeCollection which allows you to add an IEnumerable<TreeNode>. It would still loop under the hood but that detail would be abstracted from you :) \$\endgroup\$ – MattDavey Oct 14 '11 at 8:27
  • \$\begingroup\$ I'd also a method BookToTreeNode(Book b) which could be used in a linq projection -- authorNode.Nodes.Add(author.Books.Select(BookToTreeNode)) \$\endgroup\$ – MattDavey Oct 14 '11 at 8:32
  • \$\begingroup\$ I thought about that at first but decided it is better to not create extension methods for that purpose. It's too bad not all collections support adding ranges (IEnumerable<T> in particular) of items but I don't see the compelling need to write an extension method to do so either as it's not something that is needed too often. \$\endgroup\$ – Jeff Mercado Oct 14 '11 at 9:45
4
\$\begingroup\$

Looks perfectly fine to me. That's the first time I've looked at your code and I was able to see quickly what it does.

There is no need to overly-compact things if it is going to make it a pain for someone else to understand in future.

\$\endgroup\$
  • \$\begingroup\$ four nested foreach loops doesn't look that great! This is a good example of the arrowhead anti-pattern \$\endgroup\$ – MattDavey Oct 14 '11 at 8:28
  • \$\begingroup\$ @MattDavey, I was also concerned about the nested loops that's why I asked the question :) \$\endgroup\$ – awaisj Oct 17 '11 at 5:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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