For vs. For Each

[Alvin Bruney [MVP]]

Typically, you use foreach to iterate throught the collection.

Adding/Removing items from the collection during this time puts the
collection into a funny mode that others may not be ready to deal
with, what if you have multiple emunerators?

That is a design and implementation issue, not a programming issue.
Iterating a container which can change during iteration is rightly handled
internally by the construct itself and not by the iterating code so there
should be no funny mode. For instance, what's to stop the internal code from
re-adjusting its contents based on the removal or addition of an item on the
fly? This is very basic functionality available in vb if memory serves me
right.

Multiple enumerators can be handled internally thru synchronization means
and this can all be hidden from the programmer so that she is not aware how
the iterating construct is implemented (good design). I think the choice to
implement this construct as readonly must have come down to efficiency over
functionality. That's the only reason I can think of.

 

[Michal Dabrowski]

Is there a performance difference between this:

\\\
Dim i As Integer
For i = 0 to myObject.Controls.Count - 1
myObject.Controls(i) = ...
Next
///

and this:

\\\
Dim ctl As Control
For Each ctl In myObject.Controls
ctl = ...
Next
///

Or is For Each just "prettier"?

They are almost identical when your collection is some sort of an array.
But if the collection is e.g. linked list, then executing .Controls(n) will
cause your app to traverse through n elements - the bigger n is the slower
it will take to find n-th element. Using enumerators (For Each) is
considerably faster here.

Best regards,
Michal Dabrowski

 

[Nick Malik]

I don't disagree with that. very good point indeed. but, the current

approach makes it impossible to perform simple tasks inherent in UI
programming (like removing multiselects in a listbox for instance). Where
such simple tasks are overly complicated, i believe the design should be
reviewed.

good point. I like the idea of a collection object that isn't read-only,
seperate from other types of collections. Didn't another thread mention a
bit of code that Ericgu put out that does exactly this?

 

[Cor Ligthert]

From this document

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dv_vstechart/html/vbtchPerfOpt.asp

The performance difference between For and For Each loops does not appear to
be significant.

I hope this helps?

Cor

 

[Jay B. Harlow [MVP - Outlook]]

Alvin,
VB for as long as I can remember (VB1, VB2, VB3, VB5, VB6, VBA) has had
trouble modifying the collection itself when you use For Each. There may
have been one or two specific collections that may have worked, or more then
likely one thought they worked, but really didn't.

The problem is the delete/insert code would need some method of notifying
(an event possible) one or more enumerators that the collection itself was
modified, this notification IMHO for the most part is too expensive to
justify adding it in all cases.

Although I do agree, it would be nice if collections had optional
Enumerators. The "fire house" version of today, which is normally used. Plus
a "safe" version that allowed modifying the collection itself while your
iterating... For example: using For Each on DataTable.Rows is not
modifiable, while using For Each on DataTable.Select is modifiable! By
modifiable means you can call DataRow.Delete or Rows.Add...

Hope this helps
Jay

Typically, you use foreach to iterate throught the collection.
Adding/Removing items from the collection during this time puts the
collection into a funny mode that others may not be ready to deal
with, what if you have multiple emunerators?

That is a design and implementation issue, not a programming issue.
Iterating a container which can change during iteration is rightly handled
internally by the construct itself and not by the iterating code so there
should be no funny mode. For instance, what's to stop the internal code from
re-adjusting its contents based on the removal or addition of an item on the
fly? This is very basic functionality available in vb if memory serves me
right.

Multiple enumerators can be handled internally thru synchronization means
and this can all be hidden from the programmer so that she is not aware how
the iterating construct is implemented (good design). I think the choice to
implement this construct as readonly must have come down to efficiency over
functionality. That's the only reason I can think of.

--
Regards,
Alvin Bruney
[ASP.NET MVP http://mvp.support.microsoft.com/default.aspx]
Got tidbits? Get it here... http://tinyurl.com/27cok

I disagree that foreach-construct being readonly is a bad thing. Not
to completely disregard Alvin's gripe, but here's my point of view.

Typically, you use foreach to iterate throught the collection.
Adding/Removing items from the collection during this time puts the
collection into a funny mode that others may not be ready to deal
with, what if you have multiple emunerators? (this is very common for
a nested foreach scenario, yes, that'd be O(n^2) ). I believe in C++
STL libraries you can remove current iterated item, but that opens a
can of worms, you always have to worry whether your current item has
been deleted by another thread.

Also, I highly disagree that the for-construct is faster than
foreach-construct. That is only true when you are talking about ARRAY
collection types. In a linked-list implementation, foreach-construct
O(1) would be faster than for-construct O(n) for iterating through a
collection. The fact that the .NET framework collections are almost
solely based on array types may make the statement correct in 90%+ of
the time, but it is not a correct statement to make generally. And,
besides, I wait for generics!

Typically, hashtables are iterated for the entire key/value pairs,
given that accessing the value of a hashtable is O(1), if you need to
iterate through a hastable, it's easier to iterate through the keys
O(n), and grabbing the value as you go O(1). But I can't think of why
anyone would be iterating through a hashtable except may be as a debug
step to see the contents of the hashtable.

jliu - www.ssw.com.au - johnliu.net

 

[Cablewizard]

I gave this a little bit of thought. I realized that the sort of coding I do is
quite different than what "most" people are doing. I perform mostly engineering
and geospatial analysis. For me, this involves many loops, and loops within
loops. Along with iterating over recordsets countless times. In a literal since,
this is data processing in the extreme, but certainly not like manual data
entry.

However, loops are used to perform some sort of search and/or work on a block of
items. By nature, they can consume a decent portion of the overall processing
time as they are oftentimes the place where much of the actual work is taking
place. Any number of smaller functions may be performed, but potentially it is
performed many if not thousands of times. In this particular thread's example,
the operator is iterating through all the controls in a collection, presumably
to do something with them. I would hazard a guess that if you compared the
overall processor time spent within the scope of the loop, it would be
significant relative to other non-loop functions.

So while what I do may in fact be much different than most others, I still stand
by my statement. Just look at the number of times people want to know how to
keep their GUI responsive while some sort of iterative process is occurring.
Forget for a moment about the design considerations of what is really happening.
Bottom line is that the iterative processing is consuming an amount of time
significant enough to be noticeable to the operator.

Since you asked, and to exemplify Alvin's comments, here is a common occurrence
for me.
(For those who don't what to read a confusing and long-winded example, stop
reading here)
I have a geospatial dataset that contains some number of polygons/regions.
I need to find any overlapping/intersecting regions and degenerate those
intersections into separate regions.
This requires iterating through every element in the dataset and compare it to
every other element.
Additionally, for every potentially intersecting element combination, you must
iterate through every combination of vertices/segments to determine
intersection.
Each combination of intersections could result in the creation of a new region.
Each new region could also intersect with subsequent existing and/or new
regions, which could also generate new regions...
Now, if when existing regions could be degenerated into sub-regions, I could
remove the existing regions from the collection and add the new regions to the
end of the collection, then theoretically I could determine all possible tests
within the scope of 1 top-level For Each loop. But instead, I must be creative
and do something like mark the existing regions for deletion within the master
collection, add the newly created regions to a separate collection. Then perform
the same iteration over the new collection, and potentially create an additional
collection, and so on. Once all combinations are resolved, then I must go back
and iterate through all of the resulting collections to recreate the master
collection. Now in practice, the resulting implementation isn't exactly like
that, but logically it is similar.

So for me, loop performance and implementation is extremely important.

Gerald

 

[Howard Kaikow]

My recollection is that MSFT claimed that .NET, for practical purposes,
eliminated the difference in speed between For and For Each, but I've not
recently tested that assertion.

 

[Cor Ligthert]

Gerald,.

I readed it completly, however in my opinion is everything what happens
between processor and memory nowadays extremely fast and that is often
forgotten. (I am not writing this about your situation)

There is a lot of looping in every program even when you try to avoid it. I
think that the code which is created by the ILS will make a lot of loops.

Looping is in my opinion the basic of good programming, and people who think
they can avoid it are mostly making even more code to process or stop the
loop. (By instance by making a test in the loop which cost of course more
than a simple change of a byte).

The performance difference of the methods can be neglected, see for that the
MSDN article I point on in the mainthread of this thread.

I find it mostly overdone how many attention people take to a loop, while
the total througput time will mostly not change.

I wrote mostly. I think that it needs forever and for you specialy to be
done well and that it needs in a lot of situations extra attention. However
when it comes to optimizing the througput, I would first look in most cases
to other parts of the program.

Just my thougth

Cor

 

[Cablewizard]

Cor,

If I understand your comments, then I completely agree.
1. Use loops appropriately.
2. Don't loop when it is not necessary.
3. Do loop when appropriate.
4. When you do use a loop, in the end it makes little difference if you use Do,
While, For Index, or For Each. My own testing has shown this to be true.
5. What you do while in the loop is much more important than the loop itself.
Make it as efficient as practical.
6. Just make sure that your overall code design and implementation is done
well/correctly.

In the end, follow Jay's advice. Try to do it right in the first place, and if
you find out it is a problem, then worry about the extra code to try to make it
faster.

Gerald

 

[David]

David Wrote

I don't disagree with that. very good point indeed. but, the current
approach makes it impossible to perform simple tasks inherent in UI
programming (like removing multiselects in a listbox for instance).

Why is that difficult?

For each item as Object In New ArrayList(ListBox1.SelectedItems)
ListBox1.Items.Remove(item)
Next

Where
such simple tasks are overly complicated, i believe the design should be
reviewed.

Well, the case where you want to iterate over the original items while
mutating the collection is always trivial, just copy the references and
enumerate the copy. I don't see why we'd need a different enumerator
for that. And the case where you want to alter the iteration based on
actions during the iteration is

a) ambiguous and generally domain-specific, so not suitable for the CLR; and
b) probably a really bad idea.

There's a deeper issue too, which I won't really get into. But IMHO
there's a tendency for .Net developers to overuse dumb collections, and
put a lot of logic into various enumerations in controller classes that
should really be handled by the collection class itself. It's hard to
avoid doing that, but I think it's a bad habit and I'm not sure I want
to see more language features that encourage it.

 

[Alvin Bruney [MVP]]

For each item as Object In New ArrayList(ListBox1.SelectedItems)

ListBox1.Items.Remove(item)
Next

maybe if you spent 10 seconds testing your code BEFORE you posted it, you
would find out what all this discussion is about!

Well, the case where you want to iterate over the original items while
mutating the collection is always trivial, just copy the references and
enumerate the copy.

for a tutorial on how references work have a look at this most excellent
article:
http://www.dotnet247.com/247reference/a.aspx?u=http://www.pobox.com/~skeet

There's a deeper issue too, which I won't really get into. But IMHO
there's a tendency for .Net developers to overuse dumb collections, and
put a lot of logic into various enumerations in controller classes that
should really be handled by the collection class itself. It's hard to
avoid doing that, but I think it's a bad habit and I'm not sure I want
to see more language features that encourage it.

I have no clue as to what you are trying to say. This apparently has no
bearing on the previous threads.
Or maybe you may want to try again AFTER reading the relevant threads.

 

[Nick Malik]

Hello David,

Your response certainly has a lot of emotion.

too bad it isn't coherent.

(that's my long winded way of say "what the heck are you talking about?")

Well, the case where you want to iterate over the original items while
mutating the collection is always trivial, just copy the references and
enumerate the copy.

And this is efficient how? If you want to do that, then do that... but
don't make my code pay for the overhead of that functionality because you
may want to do that once out of ten-thousand calls.

I don't see why we'd need a different enumerator for that.

You just described a different enumerator

There's a deeper issue too, which I won't really get into.

I was starting to hope, but then...

But IMHO

Oh darn, you went into it...

there's a tendency for .Net
developers to overuse dumb collections, and
put a lot of logic into various enumerations in controller classes that
should really be handled by the collection class itself.
It's hard to avoid doing that, but I think it's a bad habit and I'm not sure I want
to see more language features that encourage it.

So collections are a bad idea and we should all create "smart" classes that
wrap our types with logic, like how to do a sorted list, or how to do a
stack... (ignoring the debugged code for this that is in the CLR... that's
right, if you didn't write it, it isn't any good... Sorry... I forgot).

I promise not to get you started on generics.

 

[JohnLiu]

From this document

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dv_vstechart/html/vbtchPerfOpt.asp

The performance difference between For and For Each loops does not appear to
be significant.

I hope this helps?

Cor

It is not significant if you are working with Array-based collection
(almost all .NET collections are array based). Because an enumerator
in an array collection is pretty much just an index on a particular
position.

If you are working with Linked List-based collections, performance
with enumerators O(1) are vastly superior than index-based O(n), over
large collections this is easily visible.

jliu - www.ssw.com.au - johnliu.net

 

[David]

Hello David,

Your response certainly has a lot of emotion.

It really doesn't.

too bad it isn't coherent.

(that's my long winded way of say "what the heck are you talking about?")


And this is efficient how? If you want to do that, then do that... but
don't make my code pay for the overhead of that functionality because you
may want to do that once out of ten-thousand calls.

It's not particularly efficient, but you only have two choices if you
want to mutate the collection: either copy the references or keep track
of the changes to the collection (with an event or something). Neither
is as efficient as keeping the collection read-only, and which is more
efficient depends entirely on what you're doing during the enumeration.

And if you want to enumerate over only the original items in the
collection, copying the references is your only reasonable choice. From
the point of view of efficiency, it doesn't matter whether you do this
explicitly or if .Net does it implicitly behind the scenes with a new
enumerator type.

For the example given, removing selected items from a ListBox, copying
the references is going to take a trivial amount of time compared to the
time it takes to redraw the ListBox.

You just described a different enumerator

And I didn't need a new enumerator type or new language construct to
achieve the effect.

I was starting to hope, but then...

Heh, somehow I doubt you were. This last part was putting a large
design issue into a very short paragraph, so it's understandable that
it's been misunderstood.

Oh darn, you went into it...


So collections are a bad idea and we should all create "smart" classes that
wrap our types with logic, like how to do a sorted list, or how to do a
stack... (ignoring the debugged code for this that is in the CLR... that's
right, if you didn't write it, it isn't any good... Sorry... I forgot).

Well, now who's being emotional?

Obviously we should be using the collections, but IMO they should be
used as base classes or through composition much more often than they
are now. For example, I tend to use typed collections much more often
than the generic ArrayList, etc., and .Net gives me a rich set of tools
to create them with. In my experience, I'm not unusual at all in doing
this.

But once you begin to think of collections as being not just a dumb set
of objects, but a class representing of group of specific types, then
the next step is to start treating it like a full-fledged class in its
own right. And in classic OO terms, we shouldn't be iterating over the
privates of another class to get something done, we should be sending a
message to the class to ask it to perform the action.

Alvin's example is right on target. The only unique property a
ListBoxItemCollection holds onto is whether an item is selected,
removing those items should be a public method of the collection class
(or possibly of the ListBox itself).

 

[David]

maybe if you spent 10 seconds testing your code BEFORE you posted it, you
would find out what all this discussion is about!

I did. The code works. Why do you think it doesn't? Out of curiosity,
have you run it?

for a tutorial on how references work have a look at this most excellent
article:
http://www.dotnet247.com/247reference/a.aspx?u=http://www.pobox.com/~skeet

I may be missing something basic here (not a rare event), but for the
life of me I can't figure out what you think I'm missing. I've been to
Jon's pages quite often, BTW. I realize that for some reason we're in
the midst of fun usenet snarkiness here, but I'd appreciate it if you
could explain the error more clearly, because I really don't know what
you're getting at.

I have no clue as to what you are trying to say. This apparently has no
bearing on the previous threads.

It's only tangentially related to the thread. Sorry, I thought I made
that clear.

 

[Nick Malik]

Well, now who's being emotional?

me. I was way too low on coffee. My apologies.

--- N

 

[Alvin Bruney [MVP]]

Collection was modified; enumeration operation may not execute.
Description: An unhandled exception occurred during the execution of the
current web request. Please review the stack trace for more information
about the error and where it originated in the code.

Exception Details: System.InvalidOperationException: Collection was
modified; enumeration operation may not execute.

Source Error:


Line 50: private void Button2_Click(object sender, System.EventArgs e)
Line 51: {
Line 52: foreach(ListItem li in ListBox1.Items)
Line 53: ListBox1.Items.Remove(li);
Line 54: }


running your code with a multiselect as opposed to one selection which i
suspect you didn't do.


Server Error in '/WebApplication2' Application.
--------------------------------------------------------------------------------

Collection was modified; enumeration operation may not execute.
Description: An unhandled exception occurred during the execution of the
current web request. Please review the stack trace for more information
about the error and where it originated in the code.

Exception Details: System.InvalidOperationException: Collection was
modified; enumeration operation may not execute.

Source Error:

Line 50: private void Button2_Click(object sender, System.EventArgs e)
Line 51: {
Line 52: foreach(ListItem li in ListBox1.Items)
Line 53: if(li.Selected)
Line 54: ListBox1.Items.Remove(li);

 

[Jon Skeet [C# MVP]]

Collection was modified; enumeration operation may not execute.
Description: An unhandled exception occurred during the execution of the
current web request. Please review the stack trace for more information
about the error and where it originated in the code.

Exception Details: System.InvalidOperationException: Collection was
modified; enumeration operation may not execute.

Source Error:


Line 50: private void Button2_Click(object sender, System.EventArgs e)
Line 51: {
Line 52: foreach(ListItem li in ListBox1.Items)
Line 53: ListBox1.Items.Remove(li);
Line 54: }

running your code with a multiselect as opposed to one selection which i
suspect you didn't do.

That's nothing like the code that David posted, however. Try:

foreach (ListItem li in new ArrayList(ListBox1.Items))
{
ListBox1.Items.Remove(li);
}

 

[David]

Collection was modified; enumeration operation may not execute.
Description: An unhandled exception occurred during the execution of the
current web request. Please review the stack trace for more information
about the error and where it originated in the code.

Exception Details: System.InvalidOperationException: Collection was
modified; enumeration operation may not execute.

Source Error:


Line 50: private void Button2_Click(object sender, System.EventArgs e)
Line 51: {
Line 52: foreach(ListItem li in ListBox1.Items)
Line 53: ListBox1.Items.Remove(li);
Line 54: }

Now, with all that in mind, let's go back a couple of posts and look at
the code I actually posted....

You're iterating over the original collection, I am not. Now, I read
this in the VB.Net group so I just posted as VB, so I apologize if the
translation to C# threw you off, it's hard to know which group people
are posting from.

Try...

foreach(Object o in new ArrayList(ListBox1.SelectedItems))
{
ListBox1.Items.Remove(o);
}

running your code with a multiselect as opposed to one selection which i
suspect you didn't do.

No, you either had trouble converting the code or just didn't read it
closely enough, I'm not sure which. Anyway, now that you see the
correct code hopefully my comments will make a little more sense to you.

 

[David]

me. I was way too low on coffee. My apologies.

Heh, caffeine deprivation strikes us all at some point...