Refactoring this legacy application to a cleaner state (continued) This is continued from here:

Refactor a legacy application to a less-unwieldy state

I am refactoring a legacy application. I DO NOT have the ability to change the Permissions, CustomerReport or the ListItems classes. They are in external dlls that are given to me. I realize that the ref usage is dumb and it makes NO SENSE. I am going to work with my peers to follow better practices.

This whole class looks ugly and mind you I have refactored some of it out into methods but it still is too large and unwieldy.

The code is used to return a TreeViewItem in VB. We are rewriting the front end into C# and I am using an API call to get the data from this service. So what I need is to hand the front end something reusable for every call to this report service.

I am mainly focused on cleaning up the AssembleNodes method, but any and all help is great.

public class ReportService : IReportService
{
    private readonly Permissions _permissions;

    public ReportService()
    {
        _permissions = new Permissions();
    }

    public IList&lt;Node&gt; GetFor(string userGroup)
    {
        var listItems = new List&lt;ListItem&gt;();
        //Yes this is silly since statusReturn is never used
        var statusReturn = _permissions.GetReportsForUserGroupName(userGroup, ref listItems);
        return AssembleNodes(listItems);
    }

    public IList&lt;Node&gt; GetFor(int userKey)
    {
        var listItems = new List&lt;ListItem&gt;();
        //Yes this is silly since statusReturn is never used
        var statusReturn = _permissions.GetReportsForUser(userKey, ref listItems);
        return AssembleNodes(listItems);
    }


    //I want to refactor this method down more
    private static List&lt;Node&gt; AssembleNodes(IList&lt;ListItem&gt; listItems)
    {
        var nodes = new List&lt;Node&gt;();
        if (!listItems.Any()) return nodes;
        var initialReportType = listItems[0].value;
        var initialCrs = listItems[0].key;
        var parent = AssembleNode(initialReportType);
        var child = AssembleNode(initialCrs, parent);
        foreach (var item in listItems)
        {
            var customerReport = (CustomerReport) item.dataObject;
            var grandChild = AssembleGrandChild(child, customerReport);
            if (item.value.Equals(initialReportType))
            {
                if (!item.key.Equals(initialCrs))
                {
                    initialCrs = item.key;
                    parent.Children.Add(child);
                    child = AssembleNode(item.key, parent);
                    child.Children.Add(grandChild);
                }
                    child.Children.Add(grandChild);
            }
            else
            {
                initialReportType = item.value;
                initialCrs = item.key;
                parent.Children.Add(child);
                nodes.Add(parent);
                parent = AssembleNode(item.value);
                child = AssembleNode(item.key, parent);
                grandChild = AssembleGrandChild(child, customerReport);
                child.Children.Add(grandChild);
            }
        }
        parent.Children.Add(child);
        nodes.Add(parent);
        return nodes;
    }


    private static Node AssembleNode(string value, Node parent = null)
    {
        return parent.IsNotNull() ?
            new Node(parent) { Text = value, Value = value } :
            new Node { Text = value, Value = value };
    }

    private static Node AssembleGrandChild(Node child, CustomerReport customerReport)
    {
        return new Node(child)
        {
            Text = customerReport.FileName,
            Value = customerReport.ReportKey.ToString(),
            Title = customerReport.Description
        };
    }
}


I really want to refactor this to a better state; any suggestions?
you should move 

child.Children.Add(grandChild);


outside of your if statements like so:

    foreach (var item in gmcListItems)
    {
        var customerReport = (CustomerReport) item.dataObject;
        var grandChild = AssembleGrandChild(child, customerReport);
        if (item.value.Equals(initialReportType))
        {
            if (!item.key.Equals(initialCrs))
            {
                initialCrs = item.key;
                parent.Children.Add(child);
                child = AssembleNode(item.key, parent);
            }
        }
        else
        {
            initialReportType = item.value;
            initialCrs = item.key;
            parent.Children.Add(child);
            nodes.Add(parent);
            parent = AssembleNode(item.value);
            child = AssembleNode(item.key, parent);
            grandChild = AssembleGrandChild(child, customerReport);
        }
        child.Children.Add(grandChild); // &lt;--
    }
    parent.Children.Add(child);
    nodes.Add(parent);


Otherwise you are going to have a whole lot of duplicates.