C# Winforms: Recursive conversion from objet tree to TreeView duplicates entries

Question

I'm trying to build a winforms TreeView from a tree of classes in C#, with the ability to select an item from the TreeView, by searching it's tag from the class structure.

I've been trying to implement an Update method for redrawing the whole tree when needed in a recursive manner.

First of all: All of my classes in the base tree are derived from an abstract class called Node. The Node-class contains a name (string), _id (byte), and a list of child Nodes:

abstract class Node
{
    byte _id;
    string _name;
    List<Node> _children = new List<Node>();

    public Node()
    {
        this.ID = (byte)Iterator.GenerateID;
    }
    public void AddChild(Node child)
    {
        _children.Add(child);
    }
}

The Node constructor assigns a unique ID. (this works fine so far). Any derivatives of this class succesfully generate an ID without overriding anything.

In my main form, I have a List of root nodes: private List<Node> _root = new List<Node>(); and a variable for storing the currently selected Node: private Node _selection;

The Update method I use to step through the list of Nodes and recursively add them into the TreeView of my form; tv_scene:

private void UpdateTree()
{
    tv_scene.BeginUpdate();
    tv_scene.Nodes.Clear();
    tv_scene.Nodes.Add("Scene");
    foreach (Node n in _root)
    {
        TreeNode tn = new TreeNode(n.Name);
        tn.Tag = n.ID;
        if (n.Children.Count != 0)
        {
            AddTreeItem(n, tn);
        }
        tv_scene.Nodes[0].Nodes.Add(tn);
    }
    tv_scene.ExpandAll();
    tv_scene.EndUpdate();
}

private void AddTreeItem(Node node, TreeNode parent)
{
    TreeNode tn = new TreeNode(node.Name);
    tn.Tag = node.ID;
    parent.Nodes.Add(tn);
    foreach (Node n in node.Children)
    {
        if (n.Children.Count != 0)
        {
            AddTreeItem(n, tn);
        }
    }
    return;
}

I call UpdateTree() when an update to the TreeView is needed. In my use-case, I'm storing the unique ID in the TreeView node's "Tag" member as a string, Then later converting it back to a byte in order to search through Node tree for the object represented by the item in the TreeView. This search is also done recursively:

private Node GetNodeByID(List<Node> nodes, byte? id)
{
    foreach (Node n in nodes)
    {
        if (n.ID == id)
            return n;

        if (n.Children.Count != 0)
        {
            Node result = GetNodeByID(n.Children, id);
            if (result != null)
                return result;
        }
    }
    return null;
}

I'm calling this search function by parsing the Tag of the selected item in the TreeView back into a byte like so:_selection = GetNodeByID(_root, (byte)UInt32.Parse(tv_scene.SelectedNode.Tag.ToString())); The code works fine when adding children to the root List, but whenever I attempt to add child nodes to said items (2 levels deep.) The newly added child node shows with the same name as it's parent despite being properly initialized with a correct ID and Name. Also, If i try selecting said node with _selection = GetNodeByID(_root, (byte)UInt32.Parse(tv_scene.SelectedNode.Tag.ToString())); I get it's parent node. I also can't add any children to said node, and the tree stays a maximum of 2 levels deep.

Example:

When I print the output of both my new item function and result of GetNodeByID():

Node added: [ID:0, Name:Root0]     //Adding first node(success)
Node added: [ID:1, Name:Root1]     //Adding second node(success)
Item selected: [ID:0, Name:Root0]  //Selecting the first node with GetNodeByID(success)
Node added: [ID:2, Name:Child0]    //Adding a child to the selected node (?)
Item selected: [ID:0, Name:Root0]  //Trying to select node [ID:2, Name:Child0](fail)

I'm fairly sure the logic in my recursive methods are fundamentally flawed in some way and I'm just not seeing it. Is there anything going on here preventing me from showing & selecting nodes more than 2 branches deep?

Answer 1

This is your culprit:

// Coming from UpdateTree, ...
// node is n
// parent is new TreeNode(n.Name)
private void AddTreeItem(Node node, TreeNode parent)
{
    TreeNode tn = new TreeNode(node.Name); // == n.Name
    tn.Tag = node.ID;                      // == n.ID
    parent.Nodes.Add(tn);                  // == tn.Nodes.Add(tn), basically
    foreach (Node n in node.Children)
    {
        if (n.Children.Count != 0)   // btw: Children do not get added if 
                                     //      they do not have children?
        {
            AddTreeItem(n, tn);
        }
    }
    return;
}

So, you are adding the parent to itself once, when calling AddTreeItem from UpdateTree.

One part of the fix would be to pass the children to be added to the parent, not the parent itself.

The other one would be to fix the child-count bug.

Maybe (it always helps me) think about turning the recursion into iteration. It can't hurt for practice and if that is clearer to you, then even better.