Refactor a legacy application to a less-unwieldy state

Follow-up posted here: Refactor this method to a cleaner state (continued)

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:

```
public class ReportService : IReportService
{
private readonly Permissions _Permissions;

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

public IList GetFor(string userGroup)
{
var nodes = new List();
var ListItems = new List();

//Yes this is silly since statusReturn is never used
var statusReturn = _Permissions.GetReportsForUserGroupName(userGroup, ref ListItems);

if (ListItems.Any())
{
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))
child.Children.Add(grandChild);
else
{
initialCrs = item.key;
parent.Children.Add(child);
child = AssembleNode(item.key, parent);
child.Children.Add(grandChild);
}
else
{
initialReportType = item.value;
initia

I would put:

var ListItems = new List();
//Yes this is silly since statusReturn is never used
var statusReturn = _Permissions.GetReportsForUserGroupName(userGroup, ref ListItems);

in a method that just returns the list. So you are isolating the madness a bit.

I'm not a C# programmer, but wouldn't listItems be better than ListItems?

I honestly did not understand what the code is doing. Maybe you could explain it a little bit in your post.

It seems that you have a tree structure in there. So why transform a list which originally seems to contain a tree to another list, which should also be a tree?

StackExchange Code Review Q#55795, answer score: 4