Manipulating existing XDocument (fails)

Question

Hi there,

I got the following Code snippet from my Silverlight Application:

var messages = from message in XcurrentMsg.Descendants("message")
                               where    DateTime.Parse(message.Attribute("timestamp").Value).CompareTo(DateTime.Parse(MessageCache.Last_Cached())) > 0
                               select new
                               {
                                   ip = message.Attribute("ip").Value,
                                   timestamp = message.Attribute("timestamp").Value,
                                   text = message.Value,
                               };
                if (messages == null)
                    throw new SystemException("No new messages recorded. Application tried to access non existing resources!");

            foreach (var message in messages)
            {
                XElement temporaryElement = new XElement("message", message.text.ToString(), new XAttribute("ip", message.ip.ToString()), new XAttribute("timestamp", message.timestamp.ToString()));
                XcurrentMsg.Element("root").Element("messages").Add(temporaryElement);

                AddMessage(BuildMessage(message.ip, message.timestamp, message.text));
                msgCount++;
            }

            MessageCache.CacheXML(XcurrentMsg);
            MessageCache.Refresh();

XcurrentMsg is a XDocument fetched from my server containing messages: Structure

<root>
    <messages>
          <message ip="" timestamp=""> Text </message>
    </messages>
</root>

I want to get all "message" newer than the last time I cached the XcurrentMsg. This works fine as long as I cut out the "XElement temporaryElement" and the "XcurrentMsg.Element...." and simply use the currentMsg string as output. But I want to have "new messages" being saved in my XcurrentMsg / Cache. Now if I do not cut this part out, my Application gets awesome crazy. I think it writes infinite elements to the XcurrentMsg without stopping.

I can not figure out what's the problem.

regards,

Answer 1

This is a classic LINQ gotcha.

The variable messages holds a reference to an IEnumerable<someAnonymousType>. The mistake you are making is assuming that after the assignment to messages that all the Descendents have been enumerated and the set of someAnonymousType has been built.

In reality nothing has happened at this point. Only when you start to enumerate the set with the foreach are the Descendents enumerated and the anonymous types created. Even at the point the whole set of Descendents has not been enumerated, in fact only up until the first item that mets the where clause condition, then only one projection (the result of the select) is create at a time as the foreach loops through.

Hence the messages you add are also included in the enumeration and because that begets other messages you end in an infinite loop or at least an error complaining about your attempt to modify a collection that is being enumerated.

If you want to make sure the list of items to be enumerated is fixed before beniging the loop add ToList() to the query, this creates a List<T>.

var messages = (from message in XcurrentMsg.Descendants("message")
                           where    DateTime.Parse(message.Attribute("timestamp").Value).CompareTo(DateTime.Parse(MessageCache.Last_Cached())) > 0
                           select new
                           {
                               ip = message.Attribute("ip").Value,
                               timestamp = message.Attribute("timestamp").Value,
                               text = message.Value,
                           }).ToList();

Note no need to test messages for null in either case, it will not be null, it may be an empty enumeration or list.

Answer 2

Not sure what AddMessage does, but you should defer all additions to the XcurrentMsg until the foreach is completed. So take this:

foreach (var message in messages)
{
    XElement temporaryElement = new XElement("message", 
        message.text.ToString(), new XAttribute("ip", message.ip.ToString()), 
        new XAttribute("timestamp", message.timestamp.ToString()));
    XcurrentMsg.Element("root").Element("messages").Add(temporaryElement);

    AddMessage(BuildMessage(message.ip, message.timestamp, message.text));
    msgCount++;
}

And make it this:

List<XElement> elementsToAdd = new List<XElement>();
foreach (var message in messages)
{
    XElement temporaryElement = new XElement("message", 
        message.text.ToString(), new XAttribute("ip", message.ip.ToString()), 
        new XAttribute("timestamp", message.timestamp.ToString()));
    elementsToAdd.Add(temporaryElement);

    AddMessage(BuildMessage(message.ip, message.timestamp, message.text));
    msgCount++;
}

XcurrentMsg.Element("root").Element("messages").Add(elementsToAdd.ToArray());

Hope this helps!