Why this code does not cause error in .NET 2 & 3.5 (bug?)

A

Andrus

While iterating a collection it is not allowed to remove its element.
However code below runs *without* (!) error in .NET 2.0 and 3.5

There could be some buggy IEnumerable implementation that fails to report
errorneous
removal of collection content

Please confirm, is this .NET bug ?

Andrus.


using System;
using System.Windows.Forms;
using System.Xml;

public class Test
{
[STAThread]
static void Main()
{
try
{
XmlDocument xDoc = new XmlDocument();
xDoc.PreserveWhitespace = false;
xDoc.LoadXml(xml);
XmlNodeList list = xDoc.SelectNodes("//comment()");
foreach (XmlNode node in list)
{
node.ParentNode.RemoveChild(node);
}
}
catch (Exception ex)
{
Clipboard.SetText(ex.ToString());
MessageBox.Show(ex.ToString());
}
}

const string xml = @"<?xml version=""1.0"">
<Report><!-- --></Report>
";

}
 
C

Chris Shepherd

Andrus said:
While iterating a collection it is not allowed to remove its element.
However code below runs *without* (!) error in .NET 2.0 and 3.5

There could be some buggy IEnumerable implementation that fails to report
errorneous
removal of collection content

Please confirm, is this .NET bug ?

No, it isn't because "list" is not the same collection as
node.ParentNode.ChildNodes.


Chris.
 
A

Andrus

No, it isn't because "list" is not the same collection as
node.ParentNode.ChildNodes.

Chris,

Reply from Atsushi: It is like saying that when the IEnumerable implements
ICloneable, then a
cloned IEnumerable instance is a different collection and hence it is not
the
same collection of the IEnumerable. That makes no sense.

So this is bug.

Andrus.
 
C

Chris Shepherd

Andrus said:
Chris,

Reply from Atsushi: It is like saying that when the IEnumerable implements
ICloneable, then a
cloned IEnumerable instance is a different collection and hence it is not
the
same collection of the IEnumerable. That makes no sense.

Actually, it does. A cloned IEnumerable instance is *not* the same
IEnumerable, by nature. It may have references to the same data, but it
does not necessarily contain the same list of references. If you remove
items from the newly cloned instance, it won't affect its progenitor,
and vice versa. Kind of the point of clone, really.
So this is bug.

No, it isn't. You are functionally working with two different
IEnumerable objects. Below is an example of effectively what your
original code is doing -- removing a subset of items in one list from
another list.


Chris.



class General
{
class ClassA
{
public string name;

public ClassA(string nm) { this.name = nm; }
}

static void Main(string[] args)
{
List<ClassA> listOne = new List<ClassA>();
List<ClassA> listTwo = new List<ClassA>();

ClassA a = new ClassA("A");
ClassA b = new ClassA("B");
ClassA c = new ClassA("C");
ClassA d = new ClassA("D");
ClassA e = new ClassA("E");

listOne.AddRange(new ClassA[] { a, b, c, d, e });
listTwo.AddRange(new ClassA[] { a, c, d });

string msg = "Start Length of listOne: " +
listOne.Count.ToString() +
"\nStart Length of listTwo: " +
listTwo.Count.ToString() +
"\nWhat is removed:\n";
foreach (ClassA z in listTwo)
{
msg += z.name + "\t";
listOne.Remove(z);
}

Console.WriteLine(msg + "\nLength of listOne: " +
listOne.Count.ToString() +
"\nLength of listTwo: " +
listTwo.Count.ToString());
}
}
 
F

Family Tree Mike

Andrus said:
While iterating a collection it is not allowed to remove its element.
However code below runs *without* (!) error in .NET 2.0 and 3.5

There could be some buggy IEnumerable implementation that fails to report
errorneous
removal of collection content

Please confirm, is this .NET bug ?

Andrus.


using System;
using System.Windows.Forms;
using System.Xml;

public class Test
{
[STAThread]
static void Main()
{
try
{
XmlDocument xDoc = new XmlDocument();
xDoc.PreserveWhitespace = false;
xDoc.LoadXml(xml);
XmlNodeList list = xDoc.SelectNodes("//comment()");
foreach (XmlNode node in list)
{
node.ParentNode.RemoveChild(node);
}
}
catch (Exception ex)
{
Clipboard.SetText(ex.ToString());
MessageBox.Show(ex.ToString());
}
}

const string xml = @"<?xml version=""1.0"">
<Report><!-- --></Report>
";

}

Though it is not advised to do what you have shown, I have never known it to
be illegal/not allowed.
 
A

Andrus

Chris,
No, it isn't. You are functionally working with two different IEnumerable
objects. Below is an example of effectively what your original code is
doing -- removing a subset of items in one list from another list.

The explanation again does not make sense here because it does have
references
to the same data.

This would be in case if original code stores ALL node in a temporary list
and THEN iterates the list.
This is very much unacceptable for performance reason and original code does
not do this.

Andrus.
 
A

Andrus

Though it is not advised to do what you have shown, I have never known it
to
be illegal/not allowed.

Code removes iterated item from collection inside foreach loop.
C# clearly does not allow to change iterated collection in foreach body.
This should cause exception.
So why do you think that this can allowed ?

Andrus.
 
B

Bill Butler

Andrus said:
Code removes iterated item from collection inside foreach loop.
C# clearly does not allow to change iterated collection in foreach
body.
This should cause exception.
So why do you think that this can allowed ?

It is because the compiler doesn't look that deeply to see if you are
attempting to subvert it's operation.
Here is a simpler case that shows the same behavior.
While it might be obvious that list and alias are the same collection,
it is not obvious to the compiler

using System.Collections.Generic;

public class Test
{
public static void Main()
{
List<string> list = new List<string>();
List<string> alias = list;
list.Add("A");
list.Add("B");
list.Add("C");

foreach(string str in list)
{
if (str == "A")
alias.Remove("A");
}
}
}
 
A

Andrus

Bill,
It is because the compiler doesn't look that deeply to see if you are
attempting to subvert it's operation.
Here is a simpler case that shows the same behavior.
While it might be obvious that list and alias are the same collection, it
is not obvious to the compiler

Your code throws runtime exception.

My code does not throw runtime exception.
Is this XML DOM Enumerator bug ?

How to remove all comments from XML string properly (rewrite this code) ?

Andrus.
 
J

Jon Skeet [C# MVP]

My code does not throw runtime exception.
Is this XML DOM Enumerator bug ?

No. From the docs for XmlDocument.SelectNodes:

<quote>
An XmlNodeList containing a collection of nodes matching the XPath
query. The XmlNodeList should not be expected to be connected "live" to
the XML document. That is, changes that appear in the XML document may
not appear in the XmlNodeList, and vice versa.
</quote>
 
A

Andrus

No. From the docs for XmlDocument.SelectNodes:
<quote>
An XmlNodeList containing a collection of nodes matching the XPath
query. The XmlNodeList should not be expected to be connected "live" to
the XML document. That is, changes that appear in the XML document may
not appear in the XmlNodeList, and vice versa.
</quote>

As I understand, this code may work in some cases and in some other cases
(like using other data or other version of .NET) it may cause exception.

Why .NET allows to use such broken code ?
Has java 6 also such kind on desing flaw or is it better designed ?

Andrus.
 
S

Serge Baltic

Hello,
Why .NET allows to use such broken code ?

Why do you consider this code to be “broken� Would you like the code sample
that clones a list, iterates the second one and removes items from the first
one to throw exceptions also?

(H) Serge
 
J

Jon Skeet [C# MVP]

Andrus said:
As I understand, this code may work in some cases and in some other cases
(like using other data or other version of .NET) it may cause exception.

Yes. In other words, you shouldn't depend on it working either way.
Why .NET allows to use such broken code ?

I don't see it as being particularly broken.
Has java 6 also such kind on desing flaw or is it better designed ?

There are many ways in which .NET is better designed than Java. There
are a few examples the other way round, of course, but generally I
prefer the design of .NET.
 
C

Chris Shepherd

Andrus said:
The explanation again does not make sense here because it does have
references to the same data.

Yes, but the lists themselves are separate.
This would be in case if original code stores ALL node in a temporary list
and THEN iterates the list.
This is very much unacceptable for performance reason and original code does
not do this.

Yes, it does, it's just not exposing it to you. What performance issues
are you encountering that leads you to say this is unacceptable?

Chris.
 
Top