Anonymous methods, factory methods, threading, and XML input

Question

I have an application which retrieves data from a (potentially) very large XML file. A combination of XMLReader and XDocument seemed to be the only way I could manage the entire thing the way I wanted. It works, and it does so fairly quickly and efficiently. But I have a few review questions.

• Is this combination of XMLReader and XDocument the right way to go? Nothing I read about managing XML documents suggested it, but it was the only option I could come up with since XDocument can't load entire sections, so XMLReader is needed as well.
• Activator.CreateInstance is not something I truly understand and it was just the last in a line of options I tried there. It seems to do what I want, but is there some issues I should know about?
• The reason for the distinction between factory methods and Activator.CreatInstance is because the factory methods ifs contain classes which may return subclasses, the activator else doesn't. Is there a better way of handling this?

public static void Parse(World world, string path)
{
    isFinished = false;

    World = world;
    Path = path;

    XReader = XmlReader.Create(new StreamReader(Path, Encoding.GetEncoding("ISO-8859-9")));

    XReader.Read();
    while (XReader.Read())
    {
        if (XReader.NodeType == XmlNodeType.Whitespace)
            continue;
        if (XReader.Name == "df_world")
        {
            while (XReader.Read())
            {
                if (XReader.NodeType == XmlNodeType.Whitespace)
                    continue;
                if (XReader.NodeType == XmlNodeType.EndElement && XReader.Name == "df_world")
                    break;
                bool knownSection = true;
                switch (XReader.Name)
                {
                    case "regions":
                        LoadSection<Region>(World.Regions);
                        break;
                    case "underground_regions":
                        LoadSection<UndergroundRegion>(World.UndergroundRegions);
                        break;
                    case "sites":
                        LoadSection<Site>(World.Sites);
                        break;
                    //Other cases culled for brevity
                    default:
                        Program.Log(LogType.Error, "Unknown XML Section: " + XReader.Name);
                        XReader.Skip();
                        knownSection = false;
                        break;
                }
                if (knownSection)
                    OnFinishedSection(XReader.Name);
            }
            break;
        }
    }
    isFinished = true;
    OnFinished();
}


private static void LoadSection<T>(Dictionary<int, T> WorldList) where T : XMLObject
{
    while (XReader.Read())
    {
        if (XReader.NodeType == XmlNodeType.Whitespace)
            continue;
        if (XReader.NodeType != XmlNodeType.EndElement && XReader.Depth == 2)
            LoadItem<T>(WorldList);
        else if (XReader.Depth >= 2)
            continue;
        else if (XReader.NodeType == XmlNodeType.EndElement)
            break;
        else
            Program.Log(LogType.Error, "Unknown part of section xml/n" + XReader.Name);
    }
}

private static void LoadItem<T>(Dictionary<int, T> WorldList) where T : XMLObject
{
    try
    {
        XDocument xdoc = XDocument.Load(XReader.ReadSubtree());
        if (typeof(T)  == typeof(HistoricalEvent))
        {
            HistoricalEvent evt = HistoricalEvent.Create(xdoc, World);
            World.HistoricalEvents.Add(evt.ID, evt);
        }
        else if (typeof(T) == typeof(HistoricalEventCollection))
        {
            HistoricalEventCollection evtcol = HistoricalEventCollection.Create(xdoc, World);
            World.HistoricalEventCollections.Add(evtcol.ID, evtcol);
        }
        else
        {
            T WorldObject = (T)Activator.CreateInstance(typeof(T), new object[] { xdoc, World });
            WorldList.Add(WorldObject.ID, WorldObject);
        }

    }
    catch (Exception e )
    {
        Program.Log(LogType.Error, "Error reading XML item\n" + e.Message);
    }

}

Answer 1

My first observations is: wrap disposable classes in using. The code as is probably has a memory leak in it:

using (var streamReader = new StreamReader(Path, Encoding.GetEncoding("ISO-8859-9"))
using (var xmlReader = XmlReader.Create(streamReader))
{
    // rest of logic.
}

I don't like the variable name XReader. I don't see the declaration, but it should be a local variable, and named xReader. If it is a property, I would rethink the name. Same with World and Path (I"m assuming these are class properties). Why are you saving those variables in Properties any ways? Are they really needed anywhere else?

The layout of the code is not bad, easy to read and follow. Again, the biggest issue I see is that you are not disposing of disposable objects. Please try and get that fixed.