patterncsharpMinor
Extracting an XML config file from ZIP file and parsing it
Viewed 0 times
filexmlzipconfigparsingextractingandfrom
Problem
I am currently working with an app that should load various info on a very specific zip file which contains the XML config file, using the DotNetZip library on C#, here's the code:
My question is, is there any better/more compact way of reaching same functionality?
public static void InitializeData(string configFilePath)
{
if (File.Exists(configFilePath))
{
using (ZipFile zip1 = ZipFile.Read(configFilePath))
{
zipEntriesCount = zip1.Count;
if (zip1.ContainsEntry(installConfigFileName))
{
var entries = zip1.SelectEntries(installConfigFileName);
ZipEntry[] entriesArray = new ZipEntry[entries.Count];
entries.CopyTo(entriesArray, 0);
entriesArray[0].Extract(installConfigStream);
installConfigData = StreamToString(installConfigStream);
XmlDocument xmlDoc = new XmlDocument(); // Create an XML document object
xmlDoc.Load(installConfigStream); // Load the XML document from the specified file
// Get elements
XmlNodeList XmlAppTitle = xmlDoc.GetElementsByTagName("appTitle");
XmlNodeList XmlAppCompany = xmlDoc.GetElementsByTagName("appCompany");
XmlNodeList XmlAppIconPath = xmlDoc.GetElementsByTagName("appIconPath");
XmlNodeList XmlAppLargeIconPath = xmlDoc.GetElementsByTagName("appLargeIconPath");
// Store app config data globally
// eg: globalVar = XmlVar[0].InnerText;
appTitle = XmlAppTitle[0].InnerText;
appCompany = XmlAppCompany[0].InnerText;
appIconPath = XmlAppIconPath[0].InnerText;
appLargeIconPath = XmlAppLargeIconPath[0].InnerText;
}
}
}
}My question is, is there any better/more compact way of reaching same functionality?
Solution
The repeated part of your code can be extracted into a sub-function.
Then use it like this:
This removes about a third of the inner most block.
Things to look out for:
-
What happens if the xml file does not have one of the tags you are expecting? It's likely that this block of code won't know the correct thing to do in this case. You should write the code so that when this happens, it is clear to the caller what the real issue is.
-
The C# naming convention for local variables is camelCase. While most of the variables are named correctly, the
-
There is a comment that refers to global variables. Having global mutable variables will make it harder to reason about how the code go into a certain state. It would be better to create a simple data class and have the code that parses the file return a new instance of that class with the parsed values.
string InnerTextOfFirst(XmlDocument doc, string tagName)
{
return doc.GetElementsByTagName(tagName)[0].InnerText;
}Then use it like this:
appTitle = InnerTextOfFirst(xmlDoc, "appTitle");
appCompany = InnerTextOfFirst(xmlDoc, "appCompany");
appIconPath = InnerTextOfFirst(xmlDoc, "appIconPath");
appLargeIconPath = InnerTextOfFirst(xmlDoc, "appLargeIconPath");This removes about a third of the inner most block.
Things to look out for:
-
What happens if the xml file does not have one of the tags you are expecting? It's likely that this block of code won't know the correct thing to do in this case. You should write the code so that when this happens, it is clear to the caller what the real issue is.
-
The C# naming convention for local variables is camelCase. While most of the variables are named correctly, the
XmlNodeList variables are not.-
There is a comment that refers to global variables. Having global mutable variables will make it harder to reason about how the code go into a certain state. It would be better to create a simple data class and have the code that parses the file return a new instance of that class with the parsed values.
Code Snippets
string InnerTextOfFirst(XmlDocument doc, string tagName)
{
return doc.GetElementsByTagName(tagName)[0].InnerText;
}appTitle = InnerTextOfFirst(xmlDoc, "appTitle");
appCompany = InnerTextOfFirst(xmlDoc, "appCompany");
appIconPath = InnerTextOfFirst(xmlDoc, "appIconPath");
appLargeIconPath = InnerTextOfFirst(xmlDoc, "appLargeIconPath");Context
StackExchange Code Review Q#110288, answer score: 4
Revisions (0)
No revisions yet.