2

In c#, when I want to remove some items from a list, i do it in the following way,

List<Item> itemsToBeRemoved = new List<Item>();
foreach(Item item in myList)
{
   if (IsMatching(item)) itemsToBeRemoved.Add(item);
}

foreach(Item item in itemsToBeRemoved)
{
   myList.Remove(item);
}

Is there any better way to do it?

gilbertc
  • 1,049
  • 1
  • 10
  • 19

7 Answers7

21

Well, you could call the method that does exactly what you want.

myList.RemoveAll(IsMatching);

Generally it is "better" to use the method that does exactly what you want rather than re-inventing it yourself.

nawfal
  • 70,104
  • 56
  • 326
  • 368
Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
12
myList.RemoveAll(x=> IsMatching(x));
Brandon
  • 68,708
  • 30
  • 194
  • 223
2

Here's a bit of an improvement using Linq.

var itemsToRemove = myList.Where(item => IsMatching(item)).ToList();
foreach(Item item in itemsToBeRemoved)
{
   myList.Remove(item);
}
Jake Pearson
  • 27,069
  • 12
  • 75
  • 95
1

How about reverting your logic, adding items which doesn't match and just use the result collection?

List<Item> itemsTaken = new List<Item>();
foreach(Item item in myList)
{
   if (!IsMatching(item)) itemsTaken.Add(item);
}

// use itemsTaken as if it were myList with items removed
Adrian Godong
  • 8,802
  • 8
  • 40
  • 62
1

What I used to do is the create a reverse for-loop:

for (int i=myList.Count-1; i>=0; i--)
{
  if (IsMatching(myList[i]))
    myList.RemoveAt(i);
}

But I'm sure there are more elegant ways using LINQ.

M4N
  • 94,805
  • 45
  • 217
  • 260
0

Similar to what Jakers said, and assuming you are on .Net 3:

myList = myList.Where(item => !IsMatching(item)).ToList();
Matt Grande
  • 11,964
  • 6
  • 62
  • 89
0
foreach(Item singleItem in new List<Item>(allItems))
{
  if(IsMatching(singleItem))
     allItems.Remove(singleItem);
}
ben
  • 3,126
  • 3
  • 37
  • 51