DescriptionA List<UnrelatedObects> is returned by a 3rd party webservice, which will then be transformed to a List<ArchiveDefinition>. These ArchiveDefinition objects are connected by Parent'.ArchiveNodeId == 'Child'.ParentId.The root objects TypeOfArchive property will always have the value ArchiveType.Archive.The goal of the given class below is to build a List<ArchiveTreeNode> of these flat object list to fill a treeview control.  The class in questionpublic class ArchiveBuilder{    public static List<ArchiveTreeEntry> Build(List<ArchiveDefinition> entries)    {        List<ArchiveTreeEntry> rootArchiveTreeEntries = new List<ArchiveTreeEntry>();        if (entries != null && entries.Count > 0)        {            List<ArchiveDefinition> rootEntries = GetRootEntries(entries);            foreach (ArchiveDefinition definition in rootEntries)            {                rootArchiveTreeEntries.Add(new ArchiveTreeEntry(definition));                entries.Remove(definition);            }            foreach (ArchiveTreeEntry parent in rootArchiveTreeEntries)            {                FillChildren(parent, entries);            }        }        return rootArchiveTreeEntries;    }    private static void FillChildren(ArchiveTreeEntry parent,                                     List<ArchiveDefinition> entries)    {        if (entries.Count > 0)        {            List<ArchiveDefinition> children = GetChildren(entries, parent.Id);            if (children.Count > 0)            {                RemoveChildren(entries, parent.Id);                foreach (ArchiveDefinition child in children)                {                    ArchiveTreeEntry treeEntryChild = new ArchiveTreeEntry(child);                    parent.AddChild(treeEntryChild);                    FillChildren(treeEntryChild, entries);                }            }        }    }    private static List<ArchiveDefinition> GetRootEntries(List<ArchiveDefinition> entries)    {        return entries.FindAll(e => e.TypeOfArchive == ArchiveType.Archive);    }    private static List<ArchiveDefinition> GetChildren(List<ArchiveDefinition> entries, string parentID)    {        return entries.FindAll(e => e.ParentId == parentID);    }    private static void RemoveChildren(List<ArchiveDefinition> entries, string parentID)    {        entries.RemoveAll(e => e.ParentId == parentID);    }}Related classes and enums public class ArchiveDefinition{    public string ArchiveNodeId { get; private set; }    public string ParentId { get; private set; }    public ArchiveType TypeOfArchive { get; private set; }    public ArchiveDefinition (String parentId, String archiveNodeId,                              ArchiveType type)    {        ParentId = parentId;        TypeOfArchive = type;        ArchiveNodeId = archiveNodeId;    }}public enum ArchiveType{    Archive, ArchiveGroup, ArchiveEntry}public class ArchiveTreeEntry{    public ArchiveType ArchiveEntryType { get; private set; }    public string Id { get; private set; }    public ReadOnlyCollection<ArchiveTreeEntry> Children    {        get        {            return new ReadOnlyCollection<ArchiveTreeEntry>(mChildren);        }    }    private List<ArchiveTreeEntry> mChildren = new List<ArchiveTreeEntry>();    public ArchiveTreeEntry(ArchiveDefinition archiveDefinition)    {        Id = archiveDefinition.ArchiveNodeId;        ArchiveEntryType = archiveDefinition.TypeOfArchive;    }    internal void AddChild(ArchiveTreeEntry child)    {        if (child != null)        {            mChildren.Add(child);        }    }}I would like to get a review for the ArchiveBuilder class. If you also want to review the related classes and enums, I won't mind.
Building tree structure based on flat objects
c#;tree
I think your implementation is pretty clear. Nonetheless I tried some changes, hoping my version is more readable.I agree with Stuart's post. (using IEnumerable<> if possible, non static Build() function, ...). In Addition I think you have broken this principles:SingleResponsibility--> Your FillChildren(...) function is looking for children and removing them from the original list and adding them in the parent's children collection. I mean that are three responsibilities.LeastAttonishment--> In a function called FillChildren I doesn't expect anything being removed.I changed also many Names, but this may be a matter of taste.Here is the changed code:public class ArchiveBuilder{    public List<ArchiveTreeEntry> Build(List<ArchiveDefinition> availableArchiveDefinitions)    {        List<ArchiveTreeEntry> rootArchiveTreeEntries = null;        if (availableArchiveDefinitions != null && availableArchiveDefinitions.Count > 0)        {            rootArchiveTreeEntries = CreateRootArchiveTreeEntries(availableArchiveDefinitions);            availableArchiveDefinitions = RemoveRootArchiveDefinitions(availableArchiveDefinitions);            foreach (var entry in rootArchiveTreeEntries)            {                HandleAvailableEntriesForGivenParent(availableArchiveDefinitions, entry);            }        }        return rootArchiveTreeEntries;    }    private static void AssignChildrenToParent(ArchiveTreeEntry parent,         IEnumerable<ArchiveDefinition> children)    {        parent.AddChildRange(children.Select(x => new ArchiveTreeEntry(x)));    }    private static List<ArchiveTreeEntry> CreateRootArchiveTreeEntries(        IEnumerable<ArchiveDefinition> availableArchiveDefinitions)    {        var rootArchiveTreeEntries = new List<ArchiveTreeEntry>();        rootArchiveTreeEntries.AddRange(            availableArchiveDefinitions.Where(e => e.TypeOfArchive == ArchiveType.Archive)                .Select(x => new ArchiveTreeEntry(x)));        return rootArchiveTreeEntries;    }    private static IEnumerable<ArchiveDefinition> GetChildren(        IEnumerable<ArchiveDefinition> availableArchiveDefinitions,        string parentId)    {        return availableArchiveDefinitions.Where(e => e.ParentId == parentId);    }    private static void HandleAvailableEntriesForGivenParent(        List<ArchiveDefinition> availableArchiveDefinitions,        ArchiveTreeEntry parent)    {        if (availableArchiveDefinitions.Count > 0)        {            var children = GetChildren(availableArchiveDefinitions, parent.Id);            AssignChildrenToParent(parent, children);            RemoveAssignedItemsFromAvailabeEntries(availableArchiveDefinitions, parent.Id);            foreach (var nextParent in parent.Children)            {                HandleAvailableEntriesForGivenParent(availableArchiveDefinitions, nextParent);            }        }    }    private static void RemoveAssignedItemsFromAvailabeEntries(        List<ArchiveDefinition> availableArchiveDefinitions,        string parentId)    {        availableArchiveDefinitions.RemoveAll(e => e.ParentId == parentId);    }    private static List<ArchiveDefinition> RemoveRootArchiveDefinitions(        List<ArchiveDefinition> availableArchiveDefinitions)    {        var newEntries =            availableArchiveDefinitions.Except(                availableArchiveDefinitions.Where(e => e.TypeOfArchive == ArchiveType.Archive))                .ToList();        return newEntries;    }}