1

I've been working on an application that loads and writes configuration to XML files. I know there are some opinions about doing this, but I've been having trouble with this code.

        private static void AddToXmlTemplate(Template tmp, string _config)
    {
        string configFile = _config + "configuredTemplate.xml";
        FileStream fs = new FileStream(configFile, FileMode.OpenOrCreate);
        if (File.Exists(configFile)) {
            XDocument xD = new XDocument();
            xD.Add(new XElement("Store",
                new XElement("template",
                new XElement("filePath", tmp.TempPath),
                new XElement("Name", tmp.TempName),
                new XElement("description", tmp.TempDesc))));
            xD.Save(fs);
            fs.Flush();
            fs.Dispose();
            //commenting for change to allow sync.
        }
       else
        {
            /********------ appends the template to the config file.------*************/
            XDocument xD = XDocument.Load(fs);
            XElement root = xD.Element("Store");
            IEnumerable<XElement> rows = root.Descendants("template");
            XElement last = rows.Last();
            last.AddAfterSelf(
                new XElement("template"),
                new XElement("filePath", tmp.TempPath),
                new XElement("Name", tmp.TempName),
                new XElement("description", tmp.TempDesc));
            xD.Save(fs);
            fs.Flush();
            fs.Dispose();

        }
    }

This entire function is called in a foreach loop in another function, and all the function should to is check to see if there is a configuration file in the folder, check for html files, ask the user for information about the files, and then save out to the XML file.

I'm thinking that I need to move the Filestream manipulation, and possibly the XDocument to the calling function, and pass them into this one.

The big issue is it only saves the last set of nodes.

Jason Evans
  • 28,906
  • 14
  • 90
  • 154
Chris Rutherford
  • 1,592
  • 3
  • 22
  • 58

3 Answers3

0

I think the issue is with if (!File.Exists(configFile)). You can try this:

    private static void AddToXmlTemplate(Template tmp, string _config)
    {
        string configFile = Path.Combine(_config, "configuredTemplate.xml");
        using (FileStream fs = new FileStream(configFile, FileMode.OpenOrCreate))
        {
            if (!File.Exists(configFile))
            {
                 XElement xD = new XElement("Store",
                    new XElement("template"),
                    new XElement("filePath", tmp.TempPath),
                    new XElement("Name", tmp.TempName),
                    new XElement("description", tmp.TempDesc));
                xD.Save(fs);
                fs.Flush();
            }
            else
            {
                XDocument xD = XDocument.Load(fs);
                XElement root = xD.Element("Store");
                IEnumerable<XElement> rows = root.Descendants("template");
                XElement last = rows.Last();
                last.AddAfterSelf(
                    new XElement("template"),
                    new XElement("filePath", tmp.TempPath),
                    new XElement("Name", tmp.TempName),
                    new XElement("description", tmp.TempDesc));
                xD.Save(fs);
                fs.Flush();
            }
        }
    }
daniell89
  • 1,832
  • 16
  • 28
0

Surely the logic on your if statement is the wrong way around?

Currently if the file exists then you are creating a new xml file and you are appending it if not, it needs to be the other way around.

If you change it to this it should work

if (!File.Exists(configFile)) {
toby
  • 40
  • 5
  • there must be some other odd logic issues hanging around, because I originally had it that way, but it only ran the second block. – Chris Rutherford Apr 11 '17 at 14:53
  • Ah I think its because you are creating the filestream and then checking if it exists. Please see my other answer. – toby Apr 11 '17 at 14:56
0

Ah I think its because you are creating the filestream first and then checking if it exists, so it will always exist. If you change it to

private static void AddToXmlTemplate(Template tmp, string _config)
{
    string configFile = _config + "configuredTemplate.xml";

    if (!File.Exists(configFile)) {
        FileStream fs = new FileStream(configFile, FileMode.OpenOrCreate)
        XDocument xD = new XDocument();
        xD.Add(new XElement("Store",
            new XElement("template",
            new XElement("filePath", tmp.TempPath),
            new XElement("Name", tmp.TempName),
            new XElement("description", tmp.TempDesc))));
        xD.Save(fs);
        fs.Flush();
        fs.Dispose();
        //commenting for change to allow sync.
    }
   else
    {
        FileStream fs = new FileStream(configFile, FileMode.Open);            
/********------ appends the template to the config file.------*************/
        XDocument xD = XDocument.Load(fs);
        XElement root = xD.Element("Store");
        IEnumerable<XElement> rows = root.Descendants("template");
        XElement last = rows.Last();
        last.AddAfterSelf(
            new XElement("template"),
            new XElement("filePath", tmp.TempPath),
            new XElement("Name", tmp.TempName),
            new XElement("description", tmp.TempDesc));
        xD.Save(fs);
        fs.Flush();
        fs.Dispose();

    }
}
toby
  • 40
  • 5
  • that makes more sense, but would you be able to tell if the fs and xD variables are properly saved/disposed for the next run? – Chris Rutherford Apr 11 '17 at 15:02
  • You are disposing of the FileStream which is correct. Another way to do it is to use "using" (http://stackoverflow.com/questions/212198/what-is-the-c-sharp-using-block-and-why-should-i-use-it) – toby Apr 11 '17 at 15:09
  • thanks! now I just have to fix the issue where the update block adds a new XML declaration item. – Chris Rutherford Apr 11 '17 at 15:22
  • Do you mean it is adding an additional top level or – toby Apr 11 '17 at 15:29
  • yeah, but I think it was more how I had written some of it... not sure. Also there are issues with the Template tag is empty on the else block, but I believe I have that fixed. – Chris Rutherford Apr 11 '17 at 15:31