Return to Revisions

Well your provided example data can't be real, because I never ever saw an int? which could take 1.1 as value.

I will apply to the posted code the standard C# naming convention, meaning that properties are named using PascalCase casing.

That being said, lets take a look at the Tree class

public class Tree
{
    public int? id { get; set; }
    public string text { get; set; }
    public List<Tree> children { get; set; }
}  

which seems to be just a TreeNode with children and without a parent. IMO this is too much. Just add your TreeNode to a List<TreeNode> Children property and you won't need the Tree class anymore.

• By initializing the Children property at creation of the object, you will waste some space (memory) but you won't need to check if Children == null anymore.

• If the root node (parent == null) won't be the first node in the collection then your method will fail.

• Omitting braces although they might be optional is dangerous because this can lead to hidden and therefore hard to find bugs.

  I would like to encourage you to always use them.

• Using recursion to tackle this problem is a good idea.

###Another approach

which assumes the TreeNode looks like so

public class TreeNode
{
    public int? Id { get; set; }
    public string Text { get; set; }
    public int? Parent { get; set; }
    public List<TreeNode> Children { get; set; }
    
    public TreeNode()
    {
        Children = new List<TreeNode>();
    }
}  

where I would usually make the setters private and fill the properties at constructor level, but I will leave this for you to do.

private static TreeNode RemoveRootNode(this List<TreeNode> nodes)
{
    if (nodes == null)
    {
        throw new NullReferenceException("nodes");
    }

    var root = nodes.FirstOrDefault(n => !n.Parent.HasValue);
    if (root != null)
    {
        nodes.Remove(root);
    }

    return root;
}  

public static TreeNode BuildTree(this List<TreeNode> nodes)
{
    var root = nodes.RemoveRootNode();
    if (root == null) { throw new ArgumentOutOfRangeException("nodes"); }
    return root.BuildTree(nodes);
}  

Based on the changed example data, we now can have multiple TreeNode with Parent == null which makes the RemoveRootNode() method superfluous and will result in TreeNode buildTree(this List<TreeNode>) like so

public static TreeNode BuildTree(this List<TreeNode> nodes)
{
    if (nodes == null)
    {
        throw new ArgumentNullException("nodes");
    }
    return new TreeNode().BuildTree(nodes);
}

As we see there should be a BuildTree(this TreeNode, List<TreeNode>) method which will look like:

private static TreeNode BuildTree(this TreeNode root, List<TreeNode> nodes)
{
    if (nodes.Count == 0) { return root; }

    var children = root.FetchChildren(nodes).ToList();
    root.Children.AddRange(children);
    root.RemoveChildren(nodes);
                
    for (int i = 0; i < children.Count; i++)
    {
        children[i] = children[i].BuildTree(nodes);
        if (nodes.Count == 0) { break; }
    }

    return root;
} 

which is self-explanatory. It fetches the children by using

public static IEnumerable<TreeNode> FetchChildren(this TreeNode root, List<TreeNode> nodes)
{
    return nodes.Where(n => n.Parent == root.Id);
}  

and add them to the Children property of the root node and removes them from the nodes by

public static void RemoveChildren(this TreeNode root, List<TreeNode> nodes)
{
    foreach (var node in root.Children)
    {
        nodes.Remove(node);
    }
}

Then it iterates over the the children and calls itself recursively.