XmlReader behaves different with line breaks

Question

If the data is on a single line the index=int.Parse(logDataReader.ReadElementContentAsString()); and value=double.Parse(logDataReader.ReadElementContentAsString(), cause the cursor to move forward. If I take those calls out I see it loop 6 times in debug.

In the following only 3 <data> are read (and they are wrong as the value is for the next index) on the first (<logData id="Bravo">). On the second (<logData id="Bravo">) all <data> are read.

It is not an option to edit the xml and put in line breaks as that file is created dynamically (by XMLwriter). The NewLineChars setting is a line feed. From XMLwriter it is actually just one line - I broke it down to figure out where it was breaking. In the browser it is displayed properly.

How to fix this?

Here is my XML:

<?xml version="1.0" encoding="utf-8"?>
<log>
   <logData id="Alpha">
      <data><index>100</index><value>150</value></data>
      <data><index>110</index><value>750</value></data>
      <data><index>120</index><value>750</value></data>
      <data><index>130</index><value>150</value></data>
      <data><index>140</index><value>0</value></data>
      <data><index>150</index><value>222</value></data>
   </logData>
   <logData id="Bravo">
      <data>
         <index>100</index>
         <value>25</value>
      </data>
      <data>
         <index>110</index>
         <value>11</value>
      </data>
      <data>
         <index>120</index>
         <value>1</value>
      </data>
      <data>
         <index>130</index>
         <value>25</value></data>
      <data>
         <index>140</index>
         <value>0</value>
      </data>
      <data>
         <index>150</index>
         <value>1</value>
      </data>
   </logData>
</log>

And my code:

static void Main(string[] args)
{
    List<LogData> logDatas = GetLogDatasFromFile("singleVersusMultLine.xml");
    Debug.WriteLine("Main");
    Debug.WriteLine("logData");
    foreach (LogData logData in logDatas)
    {
        Debug.WriteLine($"    logData.ID {logData.ID}");
        foreach(LogPoint logPoint in logData.LogPoints)
        {
            Debug.WriteLine($"        logData.Index {logPoint.Index}  logData.Value {logPoint.Value}");
        }
    }
    Debug.WriteLine("end");
}       
public static List<LogData> GetLogDatasFromFile(string xmlFile)
{
    List<LogData> logDatas = new List<LogData>();

    using (XmlReader reader = XmlReader.Create(xmlFile))
    {
        // move to next "logData"
        while (reader.ReadToFollowing("logData"))
        {
            var logData = new LogData(reader.GetAttribute("id"));
            using (var logDataReader = reader.ReadSubtree())
            {
                // inside "logData" subtree, move to next "data"
                while (logDataReader.ReadToFollowing("data"))
                {
                    // move to index
                    logDataReader.ReadToFollowing("index");
                    // read index
                    var index = int.Parse(logDataReader.ReadElementContentAsString());
                    // move to value
                    logDataReader.ReadToFollowing("value");
                    // read value
                    var value = double.Parse(logDataReader.ReadElementContentAsString(), CultureInfo.InvariantCulture);
                    logData.LogPoints.Add(new LogPoint(index, value));
                }
            }
            logDatas.Add(logData);
        }
    }
    return logDatas;
}

public class LogData
{
    public string ID { get; }
    public List<LogPoint> LogPoints { get; } = new List<LogPoint>();
    public LogData (string id)
    {
        ID = id;
    }
}
public class LogPoint
{
    public int Index { get; }
    public double Value { get; }
    public LogPoint ( int index, double value)
    {
        Index = index;
        Value = value;
    }
}

The whitespace gets read as an extra node, of type [`XmlNodeType.Whitespace`](https://msdn.microsoft.com/en-us/library/system.xml.xmlnodetype.aspx). If you have code that makes one too many calls to `XmlReader.Read()`, then if the XML is indented the whitespace node gets skipped, but if the XML is not indented then something important gets skipped. See e.g. [this answer](https://stackoverflow.com/a/18282052/3744182) and [this one also](https://stackoverflow.com/a/19165632/3744182) for examples of such. — dbc, Apr 28 '18 at 18:58
@dbc Thanks. Please post as an answer. Unless someone post a solution in the next couple days I will accept. What is interesting is LINQ XML does not have this problem. — paparazzo, Apr 28 '18 at 19:43

dbc · Accepted Answer · 2018-05-18T16:36:10.883

Your problem is as follows. According to the documentation for XmlReader.ReadElementContentAsString():

This method reads the start tag, the contents of the element, and moves the reader past the end element tag.

And from the documentation for XmlReader.ReadToFollowing(String):

It advances the reader to the next following element that matches the specified name and returns true if a matching element is found.

Thus, after the call to ReadElementContentAsString(), since the reader has been advanced to the next node, it might already be positioned on the next <value> or <data> node. Then when you call ReadToFollowing() this element node is skipped because the method unconditionally moves on to the next node with the correct name. But if the XML is indented then the next node immediately after the call to ReadElementContentAsString() will be an XmlNodeType.Whitespace node, protecting against this bug.

The solution is to check whether the reader is already positioned correctly after the call to ReadElementContentAsString(). First, introduce the following extension method:

public static class XmlReaderExtensions
{
    public static bool ReadToFollowingOrCurrent(this XmlReader reader, string localName, string namespaceURI)
    {
        if (reader == null)
            throw new ArgumentNullException(nameof(reader));
        if (reader.NodeType == XmlNodeType.Element && reader.LocalName == localName && reader.NamespaceURI == namespaceURI)
            return true;
        return reader.ReadToFollowing(localName, namespaceURI);
    }
}

Then modify your code as follows:

public static List<LogData> GetLogDatasFromFile(string xmlFile)
{
    List<LogData> logDatas = new List<LogData>();

    using (XmlReader reader = XmlReader.Create(xmlFile))
    {
        // move to next "logData"
        while (reader.ReadToFollowing("logData", ""))
        {
            var logData = new LogData(reader.GetAttribute("id"));
            using (var logDataReader = reader.ReadSubtree())
            {
                // inside "logData" subtree, move to next "data"
                while (logDataReader.ReadToFollowing("data", ""))
                {
                    // move to index
                    logDataReader.ReadToFollowing("index", "");
                    // read index
                    var index = XmlConvert.ToInt32(logDataReader.ReadElementContentAsString());
                    // move to value
                    logDataReader.ReadToFollowingOrCurrent("value", "");
                    // read value
                    var value = XmlConvert.ToDouble(logDataReader.ReadElementContentAsString());
                    logData.LogPoints.Add(new LogPoint(index, value));
                }
            }
            logDatas.Add(logData);
        }
    }
    return logDatas;
}

Notes:

Always prefer to use XmlReader methods in which the local name and namespace are specified separately, such as XmlReader.ReadToFollowing (String, String). When you use a method such as XmlReader.ReadToFollowing(String) which accepts a single qualified name, you are implicitly hardcoding the choice of XML prefix, which is generally not a good idea. XML parsing should be independent of prefix choice.
While you correctly parsed your double using the CultureInfo.InvariantCulture locale, it's even easier to use the methods from the XmlConvert class to handle parsing and formatting correctly.
XmlReader.ReadSubtree() leaves the XmlReader positioned on the EndElement node of the element being read, so you shouldn't need to call ReadToFollowingOrCurrent() afterwards. (Nice use of ReadSubtree() to avoid reading too little or too much by the way; by using this method one can avoid several frequent mistakes with XmlReader.)
As you have found, code that manually reads XML using XmlReader should always be unit-tested with both formatted and unformatted XML, because certain bugs will only arise with one or the other. (See e.g. this answer, this one and this one also for other examples of such.)

Working sample .Net fiddle here.

score 2 · Answer 2 · answered Apr 28 '18 at 19:49

Indeed that code (which I provided to you in your another question) is wrong. ReadToFollowing will read to the next element with this name even if it's cursor is already positioned on element with this name. When there is a whitespace - after you read index, cursor moves to that whitespace and ReadToFollowing("value") works as you expect. However, if there is no whitespace, cursor is already on value node and so ReadToFollowing("value") reads to the next "value" in subsequent "data" node.

I think the following would be a safer approach:

public static List<LogData> GetLogDatasFromFile(string xmlFile) {
    List<LogData> logDatas = new List<LogData>();

    using (XmlReader reader = XmlReader.Create(xmlFile)) {
        LogData currentData = null;
        while (reader.Read()) {
            if (reader.IsStartElement("logData")) {
                // we are positioned on start of logData
                if (currentData != null)
                    logDatas.Add(currentData);
                currentData = new LogData(reader.GetAttribute("id"));
            }
            else if (reader.IsStartElement("data")) {
                // we are on start of "data"
                // we always have "currentData" at this point                        
                Debug.Assert(currentData != null);
                reader.ReadToFollowing("index");
                var index = int.Parse(reader.ReadElementContentAsString());
                // check if we are not already on "value"
                if (!reader.IsStartElement("value"))
                    reader.ReadToFollowing("value");
                var value = double.Parse(reader.ReadElementContentAsString(), CultureInfo.InvariantCulture);
                currentData.LogPoints.Add(new LogPoint(index, value));
            }
        }

        if (currentData != null)
            logDatas.Add(currentData);
    }

    return logDatas;
}

Thanks I will test tomorrow. It so confused me when you prior code broke when I copied into the real project. — paparazzo, Apr 28 '18 at 19:57
@paparazzo well I try to test code before posting answers, but this time I made an uncaught mistake, happens. — Evk, Apr 28 '18 at 19:59
Your code worked with the data I posted. I broke out the line breaks for a cleaner presentation in the question. — paparazzo, Apr 28 '18 at 20:00
@paparazzo yes I tested on your data of course, that gave me false feeling my code is correct :) By the way, I would consider using xml serializer, because as you see, `XmlReader` is not very user friendly. Unless your xml files are big and you are forced to reading them in streamed manner. — Evk, Apr 28 '18 at 20:05
This is just a coding test for a job. I just need it to work. Not production code. — paparazzo, Apr 28 '18 at 20:17
Nice answer. It works. The other was posted first. Thank you. — paparazzo, Apr 29 '18 at 20:59

score 0 · Answer 3 · answered Apr 28 '18 at 18:05

I found a fix but to me not an acceptable answer. XMLreader should not behave differently with line breaks.

In XmlWriter this will put line breaks in the text:

XmlWriterSettings xmlWriterSettings = new XmlWriterSettings();
xmlWriterSettings.NewLineOnAttributes = true;
xmlWriterSettings.Indent = true;
using (XmlWriter xmlWriter = XmlWriter.Create(fileNameXML, xmlWriterSettings))
{

I found this here.

XmlReader behaves different with line breaks

3 Answers3