Building tree structure based on flat objects - second follow up This is a follow up on this question which is a follow up on this question
This follow up is created, because the class in question contained something, which didn't do what it should do.

Based on the suggestion for the RemoveRootArchiveDefinitions() method of the first follow up I have renamed the name of the return variable to remainingEntries and renamed the method to GetNonRootArchiveDefinitions().

Please review the class in question, if you find anything, which should be changed.

public class ArchiveBuilder
{
    public static IEnumerable&lt;ArchiveTreeEntry&gt; Build(IEnumerable&lt;ArchiveDefinition&gt; archiveDefinitions)
    {
        IEnumerable&lt;ArchiveTreeEntry&gt; rootArchiveTreeEntries = new List&lt;ArchiveTreeEntry&gt;();

        if (archiveDefinitions != null &amp;&amp; archiveDefinitions.Count() &gt; 0)
        {
            IEnumerable&lt;ArchiveDefinition&gt; localEntries = new List&lt;ArchiveDefinition&gt;(archiveDefinitions);

            rootArchiveTreeEntries = CreateRootEntries(localEntries);
            localEntries = GetNonRootArchiveDefinitions(localEntries);

            foreach (ArchiveTreeEntry rootEntry in rootArchiveTreeEntries)
            {
                HandleEntriesForParent(localEntries, rootEntry);
            }
        }

        return rootArchiveTreeEntries;
    }

    private static IEnumerable&lt;ArchiveTreeEntry&gt; CreateRootEntries(
        IEnumerable&lt;ArchiveDefinition&gt; archiveDefinitions)
    {
        List&lt;ArchiveTreeEntry&gt; rootArchiveTreeEntries = new List&lt;ArchiveTreeEntry&gt;();

        rootArchiveTreeEntries.AddRange(
            archiveDefinitions.Where(e =&gt; e.TypeOfArchive == ArchiveType.Archive)
                .Select(d =&gt; new ArchiveTreeEntry(d)));

        return rootArchiveTreeEntries;
    }

    private static IEnumerable&lt;ArchiveDefinition&gt; GetNonRootArchiveDefinitions(
        IEnumerable&lt;ArchiveDefinition&gt; archiveDefinitions)
    {
        IEnumerable&lt;ArchiveDefinition&gt; remainingEntries =
                archiveDefinitions.Except(
                archiveDefinitions.Where(e =&gt; e.TypeOfArchive == ArchiveType.Archive));

        return remainingEntries;
    }

    private static void HandleEntriesForParent(
        IEnumerable&lt;ArchiveDefinition&gt; archiveDefinitions,
        ArchiveTreeEntry parent)
    {
        if (archiveDefinitions.Count() &gt; 0)
        {
            IEnumerable&lt;ArchiveDefinition&gt; children = GetChildren(archiveDefinitions, parent.Id);

            AddChildrenToParent(parent, children);

            archiveDefinitions = GetParentlessEntries(archiveDefinitions, parent.Id);

            foreach (ArchiveTreeEntry nextParent in parent.Children)
            {
                HandleEntriesForParent(archiveDefinitions, nextParent);
            }
        }
    }

    private static IEnumerable&lt;ArchiveDefinition&gt; GetChildren(
        IEnumerable&lt;ArchiveDefinition&gt; archiveDefinitions, string parentId)
    {
        return archiveDefinitions.Where(e =&gt; e.ParentId == parentId);
    }

    private static void AddChildrenToParent(ArchiveTreeEntry parent,
        IEnumerable&lt;ArchiveDefinition&gt; children)
    {
        parent.AddChildRange(children.Select(x =&gt; new ArchiveTreeEntry(x)));
    }

    private static IEnumerable&lt;ArchiveDefinition&gt; GetParentlessEntries(
        IEnumerable&lt;ArchiveDefinition&gt; archiveDefinitions,
        string parentId)
    {
        return archiveDefinitions.Where(e =&gt; e.ParentId != parentId);
    }

}


Regarding Any() and Count() as pointed out in the last follow up:

As long as the underlying object is a ICollection or ICollection&lt;T&gt; the Count() method returns the Count property of the underlying object. Which in turn just returns an internal counter of the items contained in the collection.

See: this question and this question
As long as the underlying object is a ICollection or ICollection&lt;T&gt; the Count() method returns the Count property of the underlying object.


That's correct, but since your parameter has type IEnumerable&lt;T&gt;, and the method is public, you have no guarantee that the underlying collection is an ICollection&lt;T&gt;. Even in non-public methods, if you assume the underlying collection is an ICollection&lt;T&gt;, your method signature should reflect that; if you decide to accept the more generic IEnumerable&lt;T&gt;, then you should use Any(). Otherwise you have hidden assumptions throughout your code that the type system cannot help with.



Use of the var keyword would clean things up considerably.



In this code


private static IEnumerable&lt;ArchiveTreeEntry&gt; CreateRootEntries(
    IEnumerable&lt;ArchiveDefinition&gt; archiveDefinitions)
{
    List&lt;ArchiveTreeEntry&gt; rootArchiveTreeEntries = new List&lt;ArchiveTreeEntry&gt;();

    rootArchiveTreeEntries.AddRange(
        archiveDefinitions.Where(e =&gt; e.TypeOfArchive == ArchiveType.Archive)
            .Select(d =&gt; new ArchiveTreeEntry(d)));

    return rootArchiveTreeEntries;
}



there's no point creating a List&lt;T&gt;, you can just write

private static IEnumerable&lt;ArchiveTreeEntry&gt; CreateRootEntries(
    IEnumerable&lt;ArchiveDefinition&gt; archiveDefinitions)
{
    return archiveDefinitions.Where(e =&gt; e.TypeOfArchive == ArchiveType.Archive)
        .Select(d =&gt; new ArchiveTreeEntry(d));
}




Similarly, you can write


private static IEnumerable&lt;ArchiveDefinition&gt; GetNonRootArchiveDefinitions(
    IEnumerable&lt;ArchiveDefinition&gt; archiveDefinitions)
{
    IEnumerable&lt;ArchiveDefinition&gt; remainingEntries =
            archiveDefinitions.Except(
            archiveDefinitions.Where(e =&gt; e.TypeOfArchive == ArchiveType.Archive));

    return remainingEntries;
}



as

private static IEnumerable&lt;ArchiveDefinition&gt; GetNonRootArchiveDefinitions(
    IEnumerable&lt;ArchiveDefinition&gt; archiveDefinitions)
{
    return archiveDefinitions.Where(e =&gt; e.TypeOfArchive != ArchiveType.Archive);
}