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<SerializerContainer> AsFrames(this IEnumerable<SerializerContainer> list){
return list.FilterBy(ConfType.CONTROLITEMCONFIG);
}
public static IEnumerable<SerializerContainer> AsVariables(this IEnumerable<SerializerContainer> list)
{
return list.FilterBy(ConfType.VARIABLECONFIG);
}
public static IEnumerable<SerializerContainer> AsCommands(this IEnumerable<SerializerContainer> list)
{
return list.FilterBy(ConfType.COMMANDCONFIG);
}
public static IEnumerable<SerializerContainer> AsWindows(this IEnumerable<SerializerContainer> list)
{
return list.FilterBy(ConfType.WINDOWCONFIG);
}
private static IEnumerable<SerializerContainer> FilterBy(this IEnumerable<SerializerContainer> list, ConfType confType)
{
return list.Where(i => i.ConfigType == confType);
}
public static IEnumerable<SerializerContainer> FilterBySource(this IEnumerable<SerializerContainer> list, String source)
{
return list.Where(i => i.Source == source);
}
public static IEnumerable<SerializerContainer> FilterByProcessed(this IEnumerable<SerializerContainer> list, Boolean processed)
{
return list.Where(i => i.Processed == processed);
}
public static IEnumerable<SerializerContainer> AsChildrenOf(this IEnumerable<SerializerContainer> list,String parent)
{
return list.Where(i => i.Parent == parent);
}
Next we should refactor
private List<SerializerContainer> ListSort(List<SerializerContainer> 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<SerializerContainer> SortList(IEnumerable<SerializerContainer> listParam)
{
if (!listParam.Any()) { return Enumerable.Empty<SerializerContainer>(); }
int listParamCount = listParam.Count();
SerializerContainer[] scArray = new SerializerContainer[listParamCount];
List<SerializerContainer> imList = listParam.FilterBySource("XXX").ToList();
List<SerializerContainer> 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<CommandConfig> DeserializeCommands(IEnumerable<SerializerContainer> nonProcessedChildren)
{
List<CommandConfig> commandList = new List<CommandConfig>();
if (!nonProcessedChildren.Any()) { return commandList; }
foreach (SerializerContainer sc in SortList(nonProcessedChildren))
{
commandList.Add(DeserializeIt<CommandConfig>(sc.Config));
sc.Processed = true;
}
return commandList;
}
private List<VariableConfig> DeserializeSortedVariables(IEnumerable<SerializerContainer> nonProcessedChildren)
{
List<VariableConfig> variableList = new List<VariableConfig>();
if (!nonProcessedChildren.Any()) { return variableList; }
foreach (SerializerContainer sc in SortList(nonProcessedChildren))
{
variableList.Add(DeserializeIt<VariableConfig>(sc.Config));
sc.Processed = true;
}
return variableList;
}
Now let us refactor private ControlItemConfig[] DeserializeFrames(string parent)
private List<ControlItemConfig> DeserializeFrames(string parent, IEnumerable<SerializerContainer> nonProcessedFrames)
{
List<ControlItemConfig> cicList = new List<ControlItemConfig>();
IEnumerable<SerializerContainer> childFrames = nonProcessedFrames.AsChildrenOf(parent);
if (!childFrames.Any()) { return cicList; }
foreach (SerializerContainer sc in SortList(childFrames))
{
ControlItemConfig itemConfig = DeserializeIt<ControlItemConfig>(sc.Config);
sc.Processed = true;
cicList.Add(itemConfig);
if ((itemConfig.Type == ControlType.Frame))
{
var frameConfig = (FrameConfig)itemConfig.Instance;
if (frameConfig.ChildControls == null)
{
frameConfig.ChildControls = DeserializeFrames(sc.Parent + sc.Name, nonProcessedFrames).ToArray();
}
}
}
return cicList;
}
As you see, this method now takes a second parameter which is called nonProcessedFrames
which is basically the original list
, with FilterByProcessed(false)
in combination with AsFrames()
like shown in the refactored DeserializeChildren()
method below
private void DeserializeChildren(WindowConfig config, string parent, IEnumerable<SerializerContainer> nonProcessedList)
{
IEnumerable<SerializerContainer> nonProcessedParents = nonProcessedList.AsChildrenOf(parent);
IEnumerable<SerializerContainer> nonProcessedWindowsParents = nonProcessedParents.AsWindows();
if (!nonProcessedList.Any()
|| !nonProcessedParents.Any()
|| !nonProcessedWindowsParents.Any()) { return; }
if (config.Frame.ChildControls == null)
{
IEnumerable<SerializerContainer> nonProcessedFrames = nonProcessedList.AsFrames();
config.Frame.ChildControls = DeserializeFrames(parent, nonProcessedFrames).ToArray();
}
if (config.Commands == null)
{
IEnumerable<SerializerContainer> nonProcessedCommands = nonProcessedParents.AsCommands();
config.Commands = DeserializeCommands(nonProcessedCommands).ToArray();
}
List<WindowConfig> wcList = new List<WindowConfig>();
foreach (SerializerContainer sc in SortList(nonProcessedWindowsParents))
{
WindowConfig windowConfig = DeserializeIt<WindowConfig>(sc.Config);
sc.Processed = true;
wcList.Add(windowConfig);
DeserializeChildren(windowConfig, parent + sc.Name, nonProcessedList);
}
if (nonProcessedWindowsParents.Any())
{
config.ChildWindows = wcList.ToArray();
}
}
As I don't know how you call the DeserializeChildren()
method the first time, let us overload it, so you can call it like you did.
private void DeserializeChildren(WindowConfig config, string parent)
{
DeserializeChildren(config, parent, list.FilterByProcessed(false));
}