foreach Iterator - which is more efficient?

[Anthony Bouch]

Everything I know about looping structures says to be careful about
expressions that need to be evaluated again and again for each
test/increment of a loop.

I came across this piece of code the other day and stopped to think for a
minute about whether it was right or not.
foreach (XmlNode node in doc.SelectNodes("/root/map/")
{
//Do something
}

Am I missing something here or is this going to perform the Xpath select for
every iteration? Or is it the case that once the iterator has been retrieved
by the foreach statement that it will use the same one for each loop without
evaluating the SelectNodes statement over and over again.

If I was able to read IL - I would look at what was going on here myself.

I've been trained to think about doing this the following way.

XmlNodeList list = doc.SelectNodes("/root/map/");
foreach (XmlNode node in list)
{
//Do something
}

Enlightenment appreciated....

 

[Guest]

I'm not sure if the compiler is smart enough to optimize this, but number 2
is definitely the way to go in my book.
Peter

 

[Mattias Sjögren]

Anthony,

Or is it the case that once the iterator has been retrieved
by the foreach statement that it will use the same one for each loop without
evaluating the SelectNodes statement over and over again.
Yes.


If I was able to read IL - I would look at what was going on here myself.

You don't have to be able to read IL. Just check the C# documentation
on how the foreach statement is compiled to a single call to
GetEnumerator (usually from the IEnumerable or IEnumerable<T>
interfaces).


Mattias

 

[Yury]

Compiler generate this:

XmlDocument doc = new XmlDocument();

XmlNodeList list = doc.SelectNodes("/root/map/");

IEnumerator enumerator = list.GetEnumerator();
try
{
XmlNode node;
while (enumerator.MoveNext())
{
node = (XmlNode)enumerator.Current;
// DO SMTH
}
}
finally
{
IDisposable disposable = (IDisposable)enumerator;
if (disposable != null)
disposable.Dispose();
}

 

[Brian Gideon]

Anthony,

The following code produced the *exact* same MSIL for the foreach loop
on my C# 1.1 compiler. As you can see the SelectNodes method is only
executed once.

IEnumerator enumerator = doc.SelectNodes("/root/map/");
try
{
while (enumerator.MoveNext())
{
XmlNode node = (XmlNode)enumerator.Current;
}
}
finally
{
IDisposable disposable = enumerator as IDisposable;
if (disposable != null)
{
disposable.Dispose();
}
}

Personally, I prefer the first method you described because it keeps
the scope of local variables as narrow as possible and yet is still
very readable.

Brian

 

[Jon Skeet [C# MVP]]

I'm not sure if the compiler is smart enough to optimize this, but number 2
is definitely the way to go in my book.

Why? It's less readable, leaves an extra variable in scope for no good
reason, and won't perform any better anyway

 

[Jon Skeet [C# MVP]]

Everything I know about looping structures says to be careful about
expressions that need to be evaluated again and again for each
test/increment of a loop.

I came across this piece of code the other day and stopped to think for a
minute about whether it was right or not.
foreach (XmlNode node in doc.SelectNodes("/root/map/")
{
//Do something
}

Am I missing something here or is this going to perform the Xpath select for
every iteration? Or is it the case that once the iterator has been retrieved
by the foreach statement that it will use the same one for each loop without
evaluating the SelectNodes statement over and over again.

Exactly. If it were re-executing SelectNodes each time, you'd keep
getting the first element of the list

If I was able to read IL - I would look at what was going on here myself.

Fortunately there's the C# spec to go on as well

From the ECMA spec, section 15.8.4:

<quote>
A foreach statement of the form:

foreach (ElementType element in collection) statement

corresponds to one of two possible expansions:

If the collection expression is of a type that implements the
collection pattern (as defined above), the expansion of the foreach
statement is:

<snip - this isn't the normal case>

Otherwise; the collection expression is of a type that implements
System.IEnumerable, and the expansion of the foreach statement is:

IEnumerator enumerator =
((System.IEnumerable)(collection)).GetEnumerator();
try {
while (enumerator.MoveNext()) {
ElementType element = (ElementType)enumerator.Current;
statement;
}
}
finally {
IDisposable disposable = enumerator as System.IDisposable;
if (disposable != null) disposable.Dispose();
}

</quote>

Note that the "collection" expression is only evaluated once.

 

[Anthony Bouch]

Overwhelmed with the replies - totally clear now and slightly embarrassed
that I didn't check the documentation first.

Thanks

 

[Brian Gideon]

Anthony,

I thought it was a good question. And as far as the documentation is
concerned I understand that sometimes you just don't know what keywords
to search on to find what you want. I did a little poking around and
the only place I found that clearly explained how the foreach construct
is compiled was the C# specification (MSDN or ECMA versions).

Brian

 

[Jon Skeet [C# MVP]]

Overwhelmed with the replies - totally clear now and slightly embarrassed
that I didn't check the documentation first.

No need for embarrassment. Yes, it's documented - but unless you knew
where to go, it wasn't exactly staring you in the face.

Put it this way - it was a lot less trivial than a lot of questions we
get here