remove ArrayList[n] inside foreach() loop?

[Guest]

Hi all,

I wanted to go through each entry(?) of ArrayList and remove some particular
entry. So I tried following but it throws exception at runtime:

foreach (myEntry entry in myArrayList)
{
// do something...

if (entry.fieldA == 0)
{ // remove entry
myArrayList.Remove(entry); // exception below
} // remove entry
}

/* exception:
System.InvalidOperationException: Collection was modified; enumeration
operation may not execute.
at System.Collections.ArrayList.ArrayListEnumeratorSimple.MoveNext()
*/

So I had to create an index array to remove and remove indexed entries
outside of foreach() loop. Is this a natural way to do or is there a better
way?

Regards

Bob

 

[Ignacio Machin \( .NET/ C# MVP \)]

Hi,

Hi all,

I wanted to go through each entry(?) of ArrayList and remove some
particular
entry. So I tried following but it throws exception at runtime:

foreach (myEntry entry in myArrayList)
{
// do something...

if (entry.fieldA == 0)
{ // remove entry
myArrayList.Remove(entry); // exception below
} // remove entry
}

/* exception:
System.InvalidOperationException: Collection was modified; enumeration
operation may not execute.
at System.Collections.ArrayList.ArrayListEnumeratorSimple.MoveNext()
*/

So I had to create an index array to remove and remove indexed entries
outside of foreach() loop. Is this a natural way to do or is there a
better
way?

You cannot modify the collection you are iterating in. You have to keep
track of those elements that you want removed and remove them after the
colection is iterate. A way of doing it is
ArrayList toremove = new ...
foreach (myEntry entry in myArrayList)
{
// do something...

if (entry.fieldA == 0)
toremove.Add( entry)
}
foreach(myEntry entry in toremove )
myArrayList.Remove( entry);

 

[Nicholas Paldino [.NET/C# MVP]]

Or, if you want to prevent having to make two iterations, you could just
iterate backwards through the list (so you can consistently traverse the
items in the list as you remove them) and remove the items as needed:

for (int index = myArrayList.Count - 1; index >= 0; index--)
{
// Get the item.
myEntry entry = (myEntry) myArrayList[index];

// Check to remove.
if (entry.fieldA == 0)
{
// Remove.
myArrayList.RemoveAt(index);
}
}

 

[bob]

On Mon, 27 Aug 2007 13:44:00 -0700, bbg
Hi,
Same theme as Ignacio,
But I would be tempted to save the wanted members to a new list
eliminating the remove step.
Depends I guess on how expensive it is to make a 'myEntry' and the
ratio of good to bad.
Bob C.

 

[Ben Voigt [C++ MVP]]

On Mon, 27 Aug 2007 13:44:00 -0700, bbg
Hi,
Same theme as Ignacio,
But I would be tempted to save the wanted members to a new list
eliminating the remove step.
Depends I guess on how expensive it is to make a 'myEntry' and the
ratio of good to bad.
Bob C.

That's what System.Collections.Generic.List<T>.RemoveAll does.

Most any use of ArrayList can be replaced by List<object> if not something
more typesafe.

 

[Alvin Bruney [MVP]]

Nope, there's an easier way to do this without the hocus pocus. Here is the
proof of concepts:

using System;
using System.Collections.Generic;
using System.Text;

namespace ConsoleApplication2
{
class Program
{
static void Main(string[] args)
{
System.Collections.ArrayList arr = new
System.Collections.ArrayList();
arr.Add("1");
arr.Add("2");
arr.Add("3");

/*This throws an exception
foreach (string s in arr)
{
arr.Remove(s);
}
*/

//where as this works like a charm
Console.WriteLine(arr.Count);
foreach (string s in new System.Collections.ArrayList(arr))
{
arr.Remove(s);
}
Console.WriteLine(arr.Count);
Console.ReadKey();
}
}
}

 

[Ben Voigt [C++ MVP]]

Nope, there's an easier way to do this without the hocus pocus. Here is
the proof of concepts:

Sure, but it's the most inefficient solution posted yet.

using System;
using System.Collections.Generic;
using System.Text;

namespace ConsoleApplication2
{
class Program
{
static void Main(string[] args)
{
System.Collections.ArrayList arr = new
System.Collections.ArrayList();
arr.Add("1");
arr.Add("2");
arr.Add("3");

/*This throws an exception
foreach (string s in arr)
{
arr.Remove(s);
}
*/

//where as this works like a charm
Console.WriteLine(arr.Count);
foreach (string s in new System.Collections.ArrayList(arr))
{
arr.Remove(s);
}
Console.WriteLine(arr.Count);
Console.ReadKey();
}
}
}


--
Regards,
Alvin Bruney
------------------------------------------------------
Shameless author plug
Excel Services for .NET - MS Press
Professional VSTO 2005 - Wrox/Wiley
OWC Black Book www.lulu.com/owc

Hi all,

I wanted to go through each entry(?) of ArrayList and remove some
particular
entry. So I tried following but it throws exception at runtime:

foreach (myEntry entry in myArrayList)
{
// do something...

if (entry.fieldA == 0)
{ // remove entry
myArrayList.Remove(entry); // exception below
} // remove entry
}

/* exception:
System.InvalidOperationException: Collection was modified; enumeration
operation may not execute.
at System.Collections.ArrayList.ArrayListEnumeratorSimple.MoveNext()
*/

So I had to create an index array to remove and remove indexed entries
outside of foreach() loop. Is this a natural way to do or is there a
better
way?

Regards

Bob

 

[Alvin Bruney [MVP]]

Sure, but it's the most inefficient solution posted yet.
Did you care to test the code before making that statement?

If you did, you'd find that the "inefficiency" is not noticable for 10,000
items being removed from the collection - at least on my lap top. And that
isn't even a real world scenario anyway. You should be far more concerned
with inefficiencies and performance issues due to network bandwidth; SQL
queries; start up times; and obscure counters to track and flag items for
deletion before railing about inefficient code.

if you care to throw in systems programming as a reason for your efficiency
comment, then you wouldn't even be using arraylists in the first place as
OPs code showed.

--
Regards,
Alvin Bruney
------------------------------------------------------
Shameless author plug
Excel Services for .NET - MS Press
Professional VSTO 2005 - Wrox/Wiley
OWC Black Book www.lulu.com/owc

Nope, there's an easier way to do this without the hocus pocus. Here is
the proof of concepts:

Sure, but it's the most inefficient solution posted yet.

using System;
using System.Collections.Generic;
using System.Text;

namespace ConsoleApplication2
{
class Program
{
static void Main(string[] args)
{
System.Collections.ArrayList arr = new
System.Collections.ArrayList();
arr.Add("1");
arr.Add("2");
arr.Add("3");

/*This throws an exception
foreach (string s in arr)
{
arr.Remove(s);
}
*/

//where as this works like a charm
Console.WriteLine(arr.Count);
foreach (string s in new System.Collections.ArrayList(arr))
{
arr.Remove(s);
}
Console.WriteLine(arr.Count);
Console.ReadKey();
}
}
}


--
Regards,
Alvin Bruney
------------------------------------------------------
Shameless author plug
Excel Services for .NET - MS Press
Professional VSTO 2005 - Wrox/Wiley
OWC Black Book www.lulu.com/owc

Hi all,

I wanted to go through each entry(?) of ArrayList and remove some
particular
entry. So I tried following but it throws exception at runtime:

foreach (myEntry entry in myArrayList)
{
// do something...

if (entry.fieldA == 0)
{ // remove entry
myArrayList.Remove(entry); // exception below
} // remove entry
}

/* exception:
System.InvalidOperationException: Collection was modified; enumeration
operation may not execute.
at System.Collections.ArrayList.ArrayListEnumeratorSimple.MoveNext()
*/

So I had to create an index array to remove and remove indexed entries
outside of foreach() loop. Is this a natural way to do or is there a
better
way?

Regards

Bob

 

[Ben Voigt [C++ MVP]]

Did you care to test the code before making that statement?

I don't need to. It makes one additional memory copy vs the "naive"
solution of using indexes instead of an iterator.

That single copy of the list plus the need to test every element is already
more expensive than the straightforward, highly efficient solution of making
a new list with just the items not removed, and then your solution also
calls Remove a number of times, which not only does a linear search to find
the index of the element being removed, it also then moves the other part of
the list to close the gap.

If you did, you'd find that the "inefficiency" is not noticable for 10,000
items being removed from the collection - at least on my lap top. And that
isn't even a real world scenario anyway. You should be far more concerned
with inefficiencies and performance issues due to network bandwidth; SQL
queries; start up times; and obscure counters to track and flag items for
deletion before railing about inefficient code.

There's no reason to write an inefficient version when the framework already
provides List<T>.RemoveAll(Predicate<T>). Premature optimization deals with
rolling your own instead of using an existing function. What you suggested
was premature de-optimization -- rolling your own and ending up with worse
performance than the existing one.

 

[Alvin Bruney [MVP]]

and then your solution also calls Remove a number of times, which not only

does a linear search to find the index of the element being removed, it
also then moves the other part of the list to close the gap.

And the generics List RemoveAll(predicate) doesn't do a linear search?
According to the docs, RemoveAll(predicate) uses find whose implementation
is a O(n) search. Isn't that linear? Are the docs wrong? Are you also
implying that RemoveAll does not re-order or purge the remaining items in
the list after a delete?

There's no reason to write an inefficient version when the framework
already provides List<T>.RemoveAll(Predicate<T>). Premature optimization
deals with rolling your own instead of using an existing function. What
you suggested was premature de-optimization -- rolling your own and ending
up with worse performance than the existing one.

The big terms are distracting. What I suggested was a way to do it that
stayed true to the original intent of the code. You haven't shown how my
version is inefficient because removeall is a linear search which includes
list re-ordering underneath. Maybe next time, you should try running the
code first. I wasn't against the generic version of the code by the way
which is why I posted a reply directly to OPs original thread and not to any
reply.

--
Regards,
Alvin Bruney
------------------------------------------------------
Shameless author plug
Excel Services for .NET - MS Press
Professional VSTO 2005 - Wrox/Wiley
OWC Black Book www.lulu.com/owc

 

[Jon Skeet [C# MVP]]

And the generics List RemoveAll(predicate) doesn't do a linear search?
According to the docs, RemoveAll(predicate) uses find whose implementation
is a O(n) search. Isn't that linear? Are the docs wrong? Are you also
implying that RemoveAll does not re-order or purge the remaining items in
the list after a delete?

RemoveAll does *one* O(n) pass through the list.

Your code does one O(n) pass for each element which needs to be
removed. Big difference.

 

[Alvin Bruney [MVP]]

Your code does one O(n) pass for each element which needs to be

removed. Big difference.

Yup, big difference - they both remove 10,000 items in the collection in
well under a second. Pretty big difference eh? I'll venture a guess that OPs
container holds less than 100 items. What are we saving here? Is this
noticeable in production code? Let's not throw around terms like
'inefficient' when the context is unclear. These terms only serve to scare
developers. If I were reviewing that application for performance, I wouldn't
even focus on that piece of code, there's no bang for developer bucks
optimizing away milliseconds.

--
Regards,
Alvin Bruney
------------------------------------------------------
Shameless author plug
Excel Services for .NET - MS Press
Professional VSTO 2005 - Wrox/Wiley
OWC Black Book www.lulu.com/owc

 

[Jon Skeet [C# MVP]]

Yup, big difference - they both remove 10,000 items in the collection in
well under a second. Pretty big difference eh? I'll venture a guess that OPs
container holds less than 100 items. What are we saving here? Is this
noticeable in production code? Let's not throw around terms like
'inefficient' when the context is unclear. These terms only serve to scare
developers. If I were reviewing that application for performance, I wouldn't
even focus on that piece of code, there's no bang for developer bucks
optimizing away milliseconds.

You seemed to be considering your code to be linear, that's all. One
part is linear, but that's repeated a number of times.

Is it a significant inefficiency? That depends on the size of list, and
wasn't what I was addressing.

 

[Guest]

I found this way(iterate backwards) was best for me and it worked well.
Thanks Nicholas.
Bob

Or, if you want to prevent having to make two iterations, you could just
iterate backwards through the list (so you can consistently traverse the
items in the list as you remove them) and remove the items as needed:

for (int index = myArrayList.Count - 1; index >= 0; index--)
{
// Get the item.
myEntry entry = (myEntry) myArrayList[index];

// Check to remove.
if (entry.fieldA == 0)
{
// Remove.
myArrayList.RemoveAt(index);
}
}


--
- Nicholas Paldino [.NET/C# MVP]
- (e-mail address removed)

Hi,



You cannot modify the collection you are iterating in. You have to keep
track of those elements that you want removed and remove them after the
colection is iterate. A way of doing it is
ArrayList toremove = new ...
foreach (myEntry entry in myArrayList)
{
// do something...

if (entry.fieldA == 0)
toremove.Add( entry)
}
foreach(myEntry entry in toremove )
myArrayList.Remove( entry);