Return to Answer

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

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

I will apply in the posted code the c# naming convention meaning properties are named using PacalCase casing.

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

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

• By initialising 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 therfor hard to find bugs.
  I would like to encourage you to always use them.

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

###Another aproach

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

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

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);
 
}  

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

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

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

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

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);
}  

By using FirstOrDefault() we can get the root node very easily and should remove it from the nodes collection. Let us introduce a method to do this

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;
}  

Now we don't need the root node to be the first item in the collection anymore.

Next we need the "main" method which takes a List<TreeNode> as a parameter and returns a TreeNode which is the root node like so

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

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 superflous 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);
}