Removing from a collection while in a foreach with linq

Question

From what I understand, this seems to not be a safe practice...

I have a foreach loop on my list object that I am stepping through. Inside that foreach loop I am looking up records by an Id. Once I have that new list of records returned by that Id I do some parsing and add them to a new list.

What I would like to do is not step through the same Id more than once. So my thought process would be to remove it from the original list. However, this causes an error... and I understand why.

My question is... Is there a safe way to go about this? or should I restructure my thought process a bit? I was wondering if anyone had any experience or thoughts on how to solve this issue?

Here is a little pseudocode:

_myList.ForEach(x => 
{
    List<MyModel> newMyList = _myList.FindAll(y => y.SomeId == x.SomeId).ToList();

    //Here is where I would do some work with newMyList

    //Now I am done... time to remove all records with x.SomeId
    _myList.RemoveAll(y => y.SomeId == x.SomeId);
});

I know that _myList.RemoveAll(y => y.SomeId == x.SomeId); is wrong, but in theory that would kinda be what I would be looking for.

I have also toyed around with the idea of pushing the used SomeId to an idList and then have it check each time, but that seems cumbersome and was wondering if there was a nicer way to handle what I am looking to do.

Sorry if i didnt explain this that well. If there are any questions, please feel free to comment and I will answer/make edits where needed.

Why not using GroupBy? You can group your records by SomeId and then cycle on those groups — BgrWorker
– BgrWorker, Commented Jul 15, 2015 at 17:15
This may be possible. I would have to try and see if this would work for me. Thanks for the input! — scapegoat17
– scapegoat17, Commented Jul 15, 2015 at 17:18
Sorry deleted my answer. I see you mentioned maintaining a list of IDs in your question. GroupBy is much easier then maintaining a list of IDs. — thinklarge
– thinklarge, Commented Jul 15, 2015 at 17:30
@SebastianoRoncato - Thank you, this did in fact work for me. If you would like to post as an answer I would be happy to mark it as correct. — scapegoat17
– scapegoat17, Commented Jul 15, 2015 at 17:47

Ocelot20 · Accepted Answer · 2015-07-15 18:05:16Z

3

First off, using ForEach in your example isn't a great idea for these reasons.

You're right to think there are performance downsides to iterating through the full list for each remaining SomeId, but even making the list smaller every time would still require another full iteration of that subset (if it even worked).

As was pointed out in the comments, GroupBy on SomeId organizes the elements into groupings for you, and allows you to efficiently step through each subset for a given SomeId, like so:

_myList.GroupBy(x => x.SomeId)
       .Select(g => DoSomethingWithGroupedElements(g));

Jon Skeet has an excellent set of articles about how the Linq extensions could be implemented. I highly recommend checking it out for a better understanding of why this would be more efficient.

answered Jul 15, 2015 at 18:05

Ocelot20

10.8k11 gold badges57 silver badges98 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

BgrWorker · Accepted Answer · 2015-07-15 18:17:50Z

First of all, a list inside a foreach is immutable, you can't add or delete content, nor rewrite an element. There are a few ways you could handle this situation:

GroupBy

This is the method I would use. You can group your list by the property you want, and iterate through the IGrouping formed this way

var groups = list.GroupBy(x => x.yourProperty);
foreach(var group in groups)
{
//your code
}

Distinct properties list

You could also save properties in another list, and cycle through that list instead of the original one

var propsList = list.Select(x=>x.yourProperty).Distinct();
foreach(var prop in propsList)
{
    var tmpList = list.Where(x=>x.yourProperty == prop);
    //your code
}

While loop

This will actually do what you originally wanted, but performances may not be optimal

while(list.Any())
{
    var prop = list.First().yourProperty;
    var tmpList = list.Where(x=>x.yourProperty == prop);
    //your code
    list.RemoveAll(x=>x.yourProperty == prop);
}

Collectives™ on Stack Overflow

Removing from a collection while in a foreach with linq

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related