Recursive reading of a UI configuration file

In our project (Windows app, Winforms), the UI is designed based on XML configuration file. I have a program to read the UI configuration file and Parse the configuration and design the UI.

The configuration file is designed for a complex UI. The UI has Frames, and Frames can have Tabs, Tabs->Tabs-> finally one control to display the content.

Following are two methods which are heavily used recursively. Their hit count is roughly 2000 or more times.

• DeserializeChildren

• DeserializeFrames

Inside these methods we have plain old nested, multiple for loops. I try to refactor these methods but I couldn't find any alternative way to do the program other than what it is currently.

private void DeserializeChildren(WindowConfig config, string parent)
{
    if (config.Frame.ChildControls == null)
    {
        config.Frame.ChildControls = DeserializeFrames(parent);
    }

    if (config.Commands == null)
    {
        config.Commands = DeserializeCommands(parent);
    }

    List scList = list.FindAll(sc => (sc.Parent == parent) &&
                                                          (sc.Processed == false) &&
                                                          (sc.ConfigType == WINDOWCONFIG));
    List wcList = new List();

    scList = ListSort(scList);

    foreach (SerializerContainer sc in scList)
    {
        string nextParent = sc.Name;
        WindowConfig windowConfig; // = new WindowConfig();
        windowConfig = DeserializeIt(sc.Config);
        sc.Processed = true;

        wcList.Add(windowConfig);
        DeserializeChildren(windowConfig, parent + nextParent);
    }
    if (wcList.Count > 0)
    {
        config.ChildWindows = wcList.ToArray();
    }

    return;
}

The above code has a call to DeserializeFrames, which is a recursive method.

```
private ControlItemConfig[] DeserializeFrames(string parent)
{
List scList = list.FindAll(sc => (sc.Parent == parent) &&

What you did good

Generally the naming of your methods with its parameters is good.

You are using braces { } also for single line if commands which is very good.

What you can do better

In every method (except ListSort()) you are calling list.FindAll() with different parameter for only the ConfigType, so it would be good start to extract this into a separate method.

You should use IEnumerable<> instead of List<> as the former will only gather the values if accessed. See https://stackoverflow.com/a/3628705/2655508

If you don't need the numbers of items in a List or of an IEnumerable but you need to check if it has any items, you should use the .Any() method as it performs faster.

For the FindAll condition, you should always check Booleans first, then Enums and last Strings for performance.

The only nitpicking I will do for naming is for the ListSort() method. Based on the naming convention a method name should be verbs or verb phrases, so ListSort() should be SortList().

Refactoring

Let us start by extracting the calls of list.FindAll() to some extension methods.

As I don't know what datatype e.g VARIABLECONFIG is , I have added an enum ConfType which can be also replaced by an String etc.

public enum ConfType
{
    WINDOWCONFIG, CONTROLITEMCONFIG, COMMANDCONFIG, VARIABLECONFIG
}

public static IEnumerable AsFrames(this IEnumerable list){
    return list.FilterBy(ConfType.CONTROLITEMCONFIG);
}
public static IEnumerable AsVariables(this IEnumerable list)
{
    return list.FilterBy(ConfType.VARIABLECONFIG);
}
public static IEnumerable AsCommands(this IEnumerable list)
{
    return list.FilterBy(ConfType.COMMANDCONFIG);
}
public static IEnumerable AsWindows(this IEnumerable list)
{
    return list.FilterBy(ConfType.WINDOWCONFIG);
}
private static IEnumerable FilterBy(this IEnumerable list, ConfType confType)
{
    return list.Where(i => i.ConfigType == confType);
}
public static IEnumerable FilterBySource(this IEnumerable list, String source)
{
    return list.Where(i => i.Source == source);
}
public static IEnumerable FilterByProcessed(this IEnumerable list, Boolean processed)
{
    return list.Where(i => i.Processed == processed);
}
public static IEnumerable AsChildrenOf(this IEnumerable list,String parent)
{
    return list.Where(i => i.Parent == parent);
}

Next we should refactor

private List ListSort(List listParam)

As I couldn't figure out what it really does let us change the name , the type of listParam and also the return type to IEnumerable.

private IEnumerable SortList(IEnumerable listParam)
{

    if (!listParam.Any()) { return Enumerable.Empty(); }

    int listParamCount = listParam.Count();
    SerializerContainer[] scArray = new SerializerContainer[listParamCount];

    List imList = listParam.FilterBySource("XXX").ToList();
    List dmList = listParam.FilterBySource("YYY").ToList();

    imList.Sort();
    dmList.Sort();

    int idx;
    for (int i = imList.Count - 1; i >= 0; i--)
    {
        if (imList[i].Order >= listParamCount)
        {
            idx = listParamCount - 1;
        }
        else
        {
            idx = imList[i].Order - 1;
        }

        while (scArray[idx] != null)
        {
            idx--;
        }

        if (idx >= 0)
        {
            scArray[idx] = imList[i];
        }
        else
        {
            throw new ArgumentOutOfRangeException(@"The index for the configuration list sort operation is less than zero.");
        }
    }

    idx = 0;

    foreach (SerializerContainer sc in dmList)
    {
        while (scArray[idx] != null)
        {
            idx++;
        }

        scArray[idx] = sc;
    }

    return scArray.AsEnumerable();
}

Maybe it is possible to call this method once with all items of the original list. Your application would gain a lot regarding performance.

Now we can start to refactor the smaller methods which aren't called recursively.

private List DeserializeCommands(IEnumerable nonProcessedChildren)
{
    List commandList = new List();

    if (!nonProcessedChildren.Any()) { return commandList; }

    foreach (SerializerContainer sc in SortList(nonProcessedChildren))
    {
        commandList.Add(DeserializeIt(sc.Config));
        sc.Processed = true;
    }

    return commandList;
}

private List DeserializeSortedVariables(IEnumerable nonProcessedChildren)
{

    List variableList = new List();

    if (!nonProcessedChildren.Any()) { return variableList; }

    foreach (SerializerContainer sc in SortList(nonProcessedChildren))
    {
        variableList.Add(DeserializeIt(sc.Config));
        sc.Processed = true;
    }

    return variableList;
}

Now let us refactor private ControlItemConfig[] DeserializeFrames(string parent)

```
private List DeserializeFrames(string parent, IEnumerable nonProcessedFrames)
{
List cicList = new List();

IEnumerable childFrames = nonProcessedFrames.AsChildre

StackExchange Code Review Q#63160, answer score: 3