Removing items from ObservableCollection

Question

I am having a problem with ObservableCollection. I have two ObservableCollection. One is old the other new. I want to check if there are items in the old one that aren't there in the new one and remove them (from the old one).

public class Class1
{
    private ObservableCollection<String> m_Files = new ObservableCollection<String>();

    public void update()
    {
        ObservableCollection<String> tmp_Files = new ObservableCollection<String>();
        tmp_Files.Add("Foo");
        tmp_Files.Add("Bar");
    
        this.m_Files.Add("Boing");
        this.m_Files.Add("Foo");
        this.m_Files.Add("Bar"); 

        this.m_Files = RemoveObsoleteFiles(this.m_Files, tmp_Files);
    }


    private ObservableCollection<String> RemoveObsoleteFiles(ObservableCollection<String> files_OLD, ObservableCollection<String> files_NEW)
    {
        ObservableCollection<String> result = files_OLD;
        foreach (String item in files_OLD)
        {
            if (!files_NEW.Contains(item))
            {
                result.Remove(item);
            }
        }
        return result;
    }
}

When the RemoveObsoleteFiles() method is called I get an error:

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

Stepping through the code I noticed that when result is changed tmp_Files also changes.

Why is that? How do I avoid it?

You have a typo, it should be `files_NEW.Remove(item)`. – Sinatr Apr 21 '21 at 12:44 — Sinatr, Apr 21 '21 at 12:44

D Stanley · Accepted Answer · 2021-04-22T01:32:39.173

2

Stepping through the code I noticed that when result is changed tmp_Files also changes.

That's because they reference the same collection. If you don't want tmp_Files to change, then you need to make a copy of it:

ObservableCollection<String> result = new Observablecollection<String>(files_OLD)

However, I suspect that's not a problem since you don't do anything with tmp_files anyways, but it may be good practice not to modify the passed in collection, and return a modified copy.

As for the exception, you can't remove items from a collection when you're iterating over it with foreach. The simplest way to remove items while looping is to use a for loop and iterate backwards (do some research to figure out why):

for(int i=files_OLD.Count - 1; i>=0; i--)
{
    String item = files_OLD[i];
    if (!files_NEW.Contains(item))
    {
        result.Remove(item);
    }
}

edited Apr 22 '21 at 01:32

answered Apr 21 '21 at 22:32

D Stanley

149,601
11
178
240

If you're going to go to the point of doing reverse iteration for the sake of optimization, you might as well use `RemoveAt(i)`, too. :-) – StriplingWarrior Apr 21 '21 at 22:52
@StriplingWarrior it's not for optimization - if you iterate forwards and remove an item, the next item gets skipped as the iterator moves forward. But yes, `RemoveAt(i)` would work also (but you still can't iterate forwards without adjusting the index when you delete). – D Stanley Apr 22 '21 at 01:31
Well, if you're not optimizing it seems sufficient to just call `ToList()` so you're iterating over a different instance. `files_OLD.Where(f => !files_NEW.Contains(f)).ToList().ForEach(i => files_OLD.Remove(i));` – StriplingWarrior Apr 22 '21 at 14:52

Removing items from ObservableCollection

1 Answers1