Load and update XML inside the constructor

Question

My requirement is, I need to load the existing xml file from the given path by passing through command line arguments and need to update the xml file from C# code. First time I am working on the C# project. I have done given requirement but need few clarifications and optimization techniques in my code snippet.

namespace SchemaChange
{
 class FragmentUpdate
 {
 private String fragmentFileRU;
 private String fragmentFileES;
 private XmlDocument docRU;
 private XmlDocument docES;
 private String fileRU;
 private String fileES;
 public FragmentUpdate(String filename)
 {
 string result = Path.GetFileName(filename);
 fileRU = "PCPE_FRAGMENT_RU.wxs";
 fileES = "PCPE_FRAGMENT_ES.wxs";
 if (result == fileRU.Trim())
 {
 fragmentFileRU = filename;
 docRU = new XmlDocument();
 docRU.Load(fragmentFileRU);
 UpdateFragmentRU();
 }
 else if (result == fileES.Trim())
 {
 fragmentFileES = filename;
 docES = new XmlDocument();
 docES.Load(fragmentFileES);
 UpdateFragmentES();
 }
 }
 private void UpdateFragmentRU()
 {
 string nameSpace = "http://wixtoolset.org/schemas/v4/wxs";
 //Getting document root Element
 XmlElement rootElement = docRU.DocumentElement;
 rootElement.SetAttribute("xmlns", nameSpace);
 XmlElement FragmentElement = (XmlElement)rootElement.GetElementsByTagName("Fragment")[0];
 XmlElement dirRefElement = (XmlElement)FragmentElement.GetElementsByTagName("DirectoryRef")[0];
 XmlElement compElement = (XmlElement)dirRefElement.GetElementsByTagName("Component")[0];
 XmlNodeList fileNodeList = compElement.GetElementsByTagName("File");
 int count = fileNodeList.Count;
 //iterating through the count of nodes
 for (int i = 0; i < count; i++)
 {
 XmlElement fileElement = (XmlElement)fileNodeList[i];
 String srcString = fileElement.GetAttribute("src");
 if (srcString != "")
 {
 //Storing value of src attribute in source attribute
 fileElement.SetAttribute("Source", srcString);
 fileElement.RemoveAttribute("src");
 }
 }
 //Saving the document
 docRU.Save(fragmentFileRU);
 }
 private void UpdateFragmentES()
 {
 string nameSpace = "http://wixtoolset.org/schemas/v4/wxs";
 XmlElement rootElement = docES.DocumentElement;
 rootElement.SetAttribute("xmlns", nameSpace);
 XmlElement FragmentElement = (XmlElement)rootElement.GetElementsByTagName("Fragment")[0];
 XmlElement dirRefElement = (XmlElement)FragmentElement.GetElementsByTagName("DirectoryRef")[0];
 XmlElement compElement = (XmlElement)dirRefElement.GetElementsByTagName("Component")[0];
 XmlNodeList fileNodeList = compElement.GetElementsByTagName("File");
 int count = fileNodeList.Count;
 for (int i = 0; i < count; i++)
 {
 //Getting the File element
 XmlElement fileElement = (XmlElement)fileNodeList[i];
 String srcString = fileElement.GetAttribute("src");
 if (srcString != "")
 {
 //Storing value of src attribute in source attribute
 fileElement.SetAttribute("Source", srcString);
 //removing src attribute
 fileElement.RemoveAttribute("src");
 }
 }
 //Saving the document
 docES.Save(fragmentFileES);
 }
 static void Main(string[] args)
 {
 if (!(args.Length == 0))
 {
 foreach (string arg in args)
 {
 FragmentUpdate fragmentUpdate = new FragmentUpdate(arg);
 }
 }
 }
}

I am passing multiple arguments from the command line. So from the Main() function for each argument I am calling the constructor. Is it the right approach to call the constructor for each command line argument?

From the constructor I am calling the functions (UpdateFragmentRU(); and UpdateFragmentES();). Is it the correct approach or can I call these functions from the Main() function.

Any optimization required in the body of the functions UpdateFragmentRU() and UpdateFragmentES().

Can I combine these two functions and optimize?

Please provide your thoughts and suggestions and help my beginning in C#.

Answer 1

With your example there is almost no difference between the two fragments other than the passed in file name format. However I suspect you want varying behavior between the two Fragment types. You could implement an adapter styled pattern like this:

public abstract class Fragment
{
 public const string RUFragmentKey = "PCPE_FRAGMENT_RU.wxs";
 public const string ESFragmentKey = "PCPE_FRAGMENT_ES.wxs";
 protected string fragmentFile;
 protected XmlDocument document;
 public virtual string NameSpace { get; } = "http://wixtoolset.org/schemas/v4/wxs";
 protected Fragment(string fileName)
 {
 string result = Path.GetFileName(fileName).Trim();
 fragmentFile = fileName;
 document = new XmlDocument();
 document.Load(fragmentFile);
 Update();
 }
 public static Fragment Create(string fileName)
 {
 fileName = fileName.Trim();
 switch (fileName)
 {
 case RUFragmentKey:
 return new RUFragment(fileName);
 case ESFragmentKey:
 return new ESFragment(fileName);
 default:
 throw new NotSupportedException($"Fragment type '{fileName}' not supported.");
 }
 }
 public virtual void Update()
 {
 XmlElement rootElement = document.DocumentElement;
 rootElement.SetAttribute("xmlns", NameSpace);
 XmlElement FragmentElement = (XmlElement)rootElement.GetElementsByTagName("Fragment")[0];
 XmlElement dirRefElement = (XmlElement)FragmentElement.GetElementsByTagName("DirectoryRef")[0];
 XmlElement compElement = (XmlElement)dirRefElement.GetElementsByTagName("Component")[0];
 XmlNodeList fileNodeList = compElement.GetElementsByTagName("File");
 for (int i = 0; i < fileNodeList.Count; i++)
 {
 //Getting the File element
 XmlElement fileElement = (XmlElement)fileNodeList[i];
 string srcString = fileElement.GetAttribute("src");
 if (srcString != "")
 {
 //Storing value of src attribute in source attribute
 fileElement.SetAttribute("Source", srcString);
 //removing src attribute
 fileElement.RemoveAttribute("src");
 }
 }
 //Saving the document
 document.Save(fragmentFile);
 }
}

Update is virtual so derived classes can use specific implementation if needed via override.

Since there is currently zero difference between the two, the initial subclasses are plain:

public class RUFragment : Fragment
{
 internal RUFragment(string fileName)
 : base(fileName) { }
 // Override functionality and add implementation details
}
public class ESFragment : Fragment
{
 internal ESFragment(string fileName)
 : base(fileName) { }
 // Override functionality and add implementation details
}

So the main function could be something along the lines of:

static void Main(string[] args)
{
 if (!(args.Length == 0))
 {
 foreach (string arg in args)
 {
 Fragment fragment = Fragment.Create(arg);
 }
 }
}

It's impossible to know what your intent is, but maybe this will get you started in a useful direction.

• Yar: 1. Why to pass string fileName to both the functions "Fragment" and "Create" 2. I understood that from Fragment() constructor we are calling update() function and it is updating xml data for both the files. Then what is the point of checking the filename in create() function. I also didn't understand what Create() function will do 3. And also what we are doing in public class RUFragment : Fragment function.
• Parrish Husband: 1. That is the private constructor on Fragment. I added it in this example because it forces a constructor onto the subclasses. So on RUFragment and ESFragment now the constructor with a filename is required.
• Parrish Husband: 2. Fragment.Create is a static factory initialization method. You want to create a Fragment based on the filename, so you let the static Fragment.Create method determine specifically which type it returns.
• Parrish Husband: 3. Since your original functionality was identical between the two fragment types, you can see that no implementation details are needed in the RUFragment and ESFragment classes. This should change per your requirements, since I assume you have a logical reason for separating these out in the first place.