I am developing a .Net application that involves reading data from XML and writing to HTML. The methods to do these tasks are all in the same class as the data they read/write are properties of objects of that class. The problem is that while they perform their function, they are rather long winded and complex. I would appreciate some ideas as to how I could neaten these methods up.
The class, data, and other functionality of this program ought to be irrelevant, but if requested, I'll give some more information. I just don't want this one post to get out of hand.
I'll show the methods in order of complexity according to CodeMaid/McCabe. First up is the HTML writing method at 11. This doesn't actually handle writing the HTML to a file, but returns a string with the HTML in it. About the first third of the method is "boilerplate" that does minimal work, mostly just writing HTML tags. The rest does more stuff, but I understand how someone who didn't write it could find it very confusing.
private String GetHtmlToWrite()
{
StringWriter stringWriter = new StringWriter();
using (HtmlTextWriter writer = new HtmlTextWriter(stringWriter))
{
writer.RenderBeginTag(HtmlTextWriterTag.Html);
writer.RenderBeginTag(HtmlTextWriterTag.Head);
writer.RenderBeginTag(HtmlTextWriterTag.Title);
writer.Write(this.Title);
writer.RenderEndTag();
writer.AddAttribute(HtmlTextWriterAttribute.Href, ServerPath + ResetCssUrl);
writer.AddAttribute(HtmlTextWriterAttribute.Type, "text/css");
writer.AddAttribute(HtmlTextWriterAttribute.Rel, "stylesheet");
writer.RenderBeginTag(HtmlTextWriterTag.Link);
writer.RenderEndTag();
writer.AddAttribute(HtmlTextWriterAttribute.Href, ServerPath + PdfCssUrl);
writer.AddAttribute(HtmlTextWriterAttribute.Type, "text/css");
writer.AddAttribute(HtmlTextWriterAttribute.Rel, "stylesheet");
writer.RenderBeginTag(HtmlTextWriterTag.Link);
writer.RenderEndTag();
writer.RenderEndTag();
writer.RenderBeginTag(HtmlTextWriterTag.Body);
writer.RenderBeginTag(HtmlTextWriterTag.H1);
writer.Write(this.Title);
writer.RenderEndTag();
writer.RenderBeginTag(HtmlTextWriterTag.H2);
writer.Write("Standard Operating Procedure");
writer.RenderEndTag();
writer.RenderBeginTag(HtmlTextWriterTag.Table);
writer.RenderBeginTag(HtmlTextWriterTag.Tr);
int MetadataCount = 0;
for (int i = 0; i < this.Data.Count; i++)
{
if (this.Data[i].IsMetadata)
{
MetadataCount++;
}
}
int numRows = (2 + MetadataCount) / 3;
for (int i = 1; i <= 3; i++)
{
writer.AddAttribute(HtmlTextWriterAttribute.Class, "third-width");
writer.RenderBeginTag(HtmlTextWriterTag.Td);
writer.AddAttribute(HtmlTextWriterAttribute.Class, "inner");
writer.RenderBeginTag(HtmlTextWriterTag.Table);
for (int j = (i * numRows) - numRows; j < (i * numRows); j++)
{
if (j < MetadataCount)
{
writer.RenderBeginTag(HtmlTextWriterTag.Tr);
writer.RenderBeginTag(HtmlTextWriterTag.Th);
writer.Write(this.Data[j].Title);
writer.RenderEndTag();
writer.RenderBeginTag(HtmlTextWriterTag.Td);
if (!String.IsNullOrWhiteSpace(this.Data[j].Value))
{
writer.Write(this.Data[j].Value);
}
else
{
writer.Write("NA");
}
writer.RenderEndTag();
writer.RenderEndTag();
}
}
writer.RenderEndTag();
writer.RenderEndTag();
}
writer.RenderEndTag();
writer.RenderEndTag();
for (int i = MetadataCount; i < this.Data.Count; i++)
{
if (this.Data[i].Value != null)
{
writer.RenderBeginTag(HtmlTextWriterTag.Div);
writer.RenderBeginTag(HtmlTextWriterTag.H3);
writer.Write(this.Data[i].Title);
writer.RenderEndTag();
writer.RenderBeginTag(HtmlTextWriterTag.P);
string[] splitContent = Regex.Split(this.Data[i].Value, @"(?<=[<.+?>])");
foreach (string line in splitContent)
{
writer.Write(line);
}
writer.RenderEndTag();
writer.RenderEndTag();
}
}
writer.RenderEndTag();
writer.RenderEndTag();
}
return stringWriter.ToString();
}
Up next we have the first of two XML read methods. This one loads a default data template file. Its complexity is 22.
private static List<StandardOperatingProcedureData> LoadDefaultStandardOperatingProcedureData()
{
List<StandardOperatingProcedureData> data = new List<StandardOperatingProcedureData>();
if (File.Exists(ServerPath + StandardOperatingProcedureDataConfigPath))
{
using (XmlTextReader reader = new XmlTextReader(ServerPath + StandardOperatingProcedureDataConfigPath))
{
string ID = null;
string title = null;
bool? isMetadata = null;
bool? isRequired = null;
while (reader.Read())
{
if (reader.IsStartElement())
{
switch (reader.Name)
{
case "ID":
if (reader.Read())
{
ID = reader.Value.Trim();
}
break;
case "Title":
if (reader.Read())
{
title = reader.Value.Trim();
}
break;
case "IsMetadata":
if (reader.Read())
{
switch (reader.Value.Trim().ToLower())
{
case "false":
isMetadata = false;
break;
case "true":
isMetadata = true;
break;
default:
throw new FileFormatException("Error: True/false incorrectly specified for field IsMetadata of data ID " + ID + ".");
}
}
break;
case "IsRequired":
if (reader.Read())
{
switch (reader.Value.Trim().ToLower())
{
case "false":
isRequired = false;
break;
case "true":
isRequired = true;
break;
default:
throw new FileFormatException("Error: True/false incorrectly specified for field IsRequired of data ID " + ID);
}
}
break;
default:
break;
}
}
if (!String.IsNullOrWhiteSpace(ID) && !String.IsNullOrEmpty(title) && isMetadata != null && isRequired != null)
{
data.Add(new StandardOperatingProcedureData(ID, title, (bool)isMetadata, (bool)isRequired));
ID = null;
title = null;
isMetadata = null;
isRequired = null;
}
}
}
}
else
{
throw new FileNotFoundException("Error: SOPData configuration file 'config/sopdata.xml' not found.");
}
return data;
}
Finally, we have the second XML reader with a complexity of 28. It reads from a file that actually stores data for an object of the class. Ideally, I'll eventually replace this with SQL storage, but for now, XML will have to do.
public void LoadStandardOperatingProcedure(String xmlToLoadFileName)
{
if (File.Exists(ServerPath + SopFolder + xmlToLoadFileName))
{
var data = new List<StandardOperatingProcedureData>();
using (XmlTextReader reader = new XmlTextReader(ServerPath + SopFolder + xmlToLoadFileName))
{
string ID = null;
string title = null;
bool? isMetadata = null;
bool? isRequired = null;
string value = null;
while (reader.Read())
{
if (reader.IsStartElement())
{
switch (reader.Name)
{
case "SOPTitle":
if (reader.Read())
{
this.Title = reader.Value.Trim();
}
break;
case "ID":
if (reader.Read())
{
ID = reader.Value.Trim();
}
break;
case "Title":
if (reader.Read())
{
title = reader.Value.Trim();
}
break;
case "IsMetadata":
if (reader.Read())
{
switch (reader.Value.Trim().ToLower())
{
case "false":
isMetadata = false;
break;
case "true":
isMetadata = true;
break;
default:
throw new FileFormatException("Error: True/false incorrectly specified for field IsMetadata of data ID " + ID + " in " + xmlToLoadFileName + ".");
}
}
break;
case "IsRequired":
if (reader.Read())
{
switch (reader.Value.Trim().ToLower())
{
case "false":
isRequired = false;
break;
case "true":
isRequired = true;
break;
default:
throw new FileFormatException("Error: True/false incorrectly specified for field IsRequired of data ID " + ID + " in " + xmlToLoadFileName + ".");
}
}
break;
case "Value":
if (reader.Read() && reader.NodeType == XmlNodeType.CDATA)
{
value = reader.Value;
}
break;
default:
break;
}
}
if (!String.IsNullOrWhiteSpace(ID) && !String.IsNullOrEmpty(title) && isMetadata != null && isRequired != null && value != null)
{
data.Add(new StandardOperatingProcedureData(ID, title, (bool)isMetadata, (bool)isRequired, value));
ID = null;
title = null;
isMetadata = null;
isRequired = null;
value = null;
}
}
}
this.Data = data;
}
else
{
throw new FileNotFoundException("SOP file " + xmlToLoadFileName + " cannot be found.");
}
}
It could be the case that the structure I am using for the XML files is needlessly long complex, so I am willing to discuss a restructuring/reformatting there. The default configuration file reads like this:
<SOPDataList>
<SOPData>
<ID>Department</ID>
<Title>Department</Title>
<IsMetadata>true</IsMetadata>
<IsRequired>true</IsRequired>
</SOPData>
...
</SOPDataList>
The actual data files read like this:
<StandardOperatingProcedure>
<SOPTitle>Standard Operating Procedure</SOPTitle>
<SOPDataList>
<SOPData>
<ID>Department</ID>
<Title>Department</Title>
<IsMetadata>True</IsMetadata>
<IsRequired>True</IsRequired>
<Value><![CDATA[Some Department]]></Value>
</SOPData>
...
</SOPDataList>
</StandardOperatingProcedure>
HtmlWriter
belong to? I was looking for something like this a little while ago. \$\endgroup\$