Removing an object from a list in a foreach loop of that list

I have made a method that eliminates any replicates of the same string in a List.

now, the problem is that it gives me this error:

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

I read in the internet, and i think that the problem is the i am removing an object from the list inside the foreach loop of the list.

    foreach (string r in list)
    {
        int numberOfAppearance=0;
        foreach (string rs in list)
        {
            if (r == rs && numberOfAppearance> 0)
                list.Remove(rs);
            else
                numberOfAppearance++;
        }
    }

How can i fix the method? Thanks for the help

Jon Skeet
people
quotationmark

Firstly, as noted in comments, LINQ has got you covered here:

list = list.Distinct().ToList();

It's well worth looking into LINQ for data operations - it can make things much simpler.

As for what's wrong with your current code - there are a couple of things:

Firstly, you're removing by item rather than by index, which will remove the first occurrence of that item, not the one you're actually looking at

Secondly, if you modify a list while you're iterating over it, you will get precisely the exception you've seen. From the docs for List<T>.GetEnumerator:

An enumerator remains valid as long as the collection remains unchanged. If changes are made to the collection, such as adding, modifying, or deleting elements, the enumerator is irrecoverably invalidated and its behavior is undefined.

You can get around this by iterating by index rather than using a foreach loop, but if you're removing an item you need to remember that everything below that will move up one element. So either you need to iterate backwards to remove items, or you need to remember to decrement the index.

Here's an approach which uses iterating by index forwards in terms of what we're looking at, but backwards in terms of looking for duplicates - stopping when we get to the index we're looking at. Note that this is still O(N2) - it's not as efficient as using Distinct:

// We're looking for duplicates *after* list[i], so we don't need to go as far
// as i being the very last element: there aren't any elements after it to be
// duplicates. (We could easily still just use list.Count, and the loop for j
// would just have 0 iterations.)
for (int i = 0; i < list.Count - 1; i++)
{
    // Go backwards from the end, looking for duplicates of list[i]
    for (int j = list.Count - 1; j > i; j--)
    {
        if (list[j] == list[i])
        {
            list.RemoveAt(j);
        }
    }
}

(For more details on Distinct, see my Edulinq post on it.)

people

See more on this question at Stackoverflow