In this method, I was tasked to take in a list of TemplateMap
and attach dynamic parameters and in a certain case, add further templates to the returned list. This functionality exists within a report rendering service and at this stage, I am constructing the templates to send into the rendering function.
ParameterName
in these templateMaps
can have the following formats:
- "XML|MPAN={}|IsCopy={}"
- "XML|MPRN={}|IsCopy={}"
- "XML"
- "XML|IsCopy={}"
FYI the ParameterName is a string and the above is how I retrieve them from the database
private static IEnumerable<TemplateMap> TransformTemplateMaps(IEnumerable<TemplateMap> templateMaps, XmlDocument xml, bool isCopy = false)
{
var transformedTemplates = new List<TemplateMap>();
var bindingOrder = 1;
foreach (var template in templateMaps)
{
var parameters = template.ParameterName.Split('|');
var skipCurrentTemplate = false;
foreach (var param in parameters)
{
var paramName = param.Split("=").First();
switch (paramName.ToUpper())
{
case "MPRN":
case "MPAN":
// Returns a list of strings
var meters = GetMetersInXml(xml, paramName);
if ((meters ?? throw new InvalidOperationException()).Count == 1)
{
template.ParameterName = template.ParameterName.Replace($"{paramName}={{}}", $"{paramName}={meters.SingleOrDefault()}");
}
else
{
// Multiple meters per site requires one template each with different meter variables
foreach (var meter in meters)
{
// Initialise clone of template instance with different parameter values
var additionalTemplate = (TemplateMap) template.Clone();
additionalTemplate.ParameterName = additionalTemplate.ParameterName.Replace($"{paramName}={{}}", $"{paramName}={meter}");
additionalTemplate.ParameterName = additionalTemplate.ParameterName.Replace($"IsCopy={{}}", $"IsCopy={isCopy}");
additionalTemplate.BindingOrder = bindingOrder;
bindingOrder++;
transformedTemplates.Add(additionalTemplate);
}
// Skips current template so that it doesn't add a duplicate template below
skipCurrentTemplate = true;
}
break;
case "ISCOPY":
template.ParameterName = template.ParameterName.Replace($"{paramName}={{}}", $"{paramName}={isCopy}");
break;
// add variables that require dynamic values here
}
}
if (skipCurrentTemplate)
continue;
template.BindingOrder = bindingOrder;
bindingOrder++;
transformedTemplates.Add(template);
}
return transformedTemplates;
}
In my code you can see that I have multiple nested foreach loops which I know to be bad design. I have found answers online that mention flattening out the nested loops into methods for better readability.
In my case however, I have certain properties i.e. bindingOrder
and skipCurrentTemplate
that need to be accessed from the inner loops so I'm not sure how to go about extracting these inner loops into methods as I'd have to return these other properties as well.
What way would you suggest I improve this code?
EDIT
Adding the TemplateMap object definition
public class TemplateMap : ICloneable
{
public string ReportPath { get; set; }
public string ParameterName { get; set; }
public int BindingOrder { get; set; }
public bool? IncludedInInvoice { get; set; }
public object Clone()
{
return new TemplateMap
{
ReportPath = this.ReportPath,
BindingOrder = this.BindingOrder,
ParameterName = this.ParameterName,
IncludedInInvoice = this.IncludedInInvoice
};
}
}
XML|MPAN={}|IsCopy={}
what isXML
? is it a validXmlDocument
? and what would{}
means ? array ? could you give us a working sample of theParameterName
I need to know the full data structure that is being processed. \$\endgroup\$XML|MPAN={}|IsCopy={}
is how the string will come from the database.XML
means that the parameter requires the standard xml for each template. If the parameter has={}
it means that it requires a dynamic value which I insert in my method. \$\endgroup\$