Modifying a Collection in a Foreach Loop C#

Question

I am having an issue with updating an item in an ObservableCollection during a foreach loop. Basically I have an ObservableCollection of employees, and they have a field in their model that decides whether or not they are in a building.

I am constantly looking at a database table to check every employee to see if there is any change in this status. This is how I do this in C#;

public ObservableCollection<EmployeeModel> EmployeesInBuilding {get; set; }
public ObservableCollection<EmployeeModel> Employees {get; set; }

var _employeeDataService = new EmployeeDataService();
EmployeesInBuilding = _employeeDataService.GetEmployeesInBuilding();
foreach (EmployeeModel empBuild in EmployeesInBuilding)
{
    foreach (EmployeeModel emp in Employees)
    {
        if (empBuild.ID == emp.ID)
        {
            if (empBuild.InBuilding != emp.InBuilding)
            {
                emp.InBuilding = empBuild.InBuilding;
                int j = Employees.IndexOf(emp);
                Employees[j] = emp;
                employeesDataGrid.Items.Refresh();
            }
        }
    }
}

This correctly picks up a change between the two ObseravbleCollections, however when I go to update the existing ObservableCollection I get an exception: Collection was modified; enumeration operation may not execute.

How can I prevent this from happening and still modify the original collection?

It doesn't make sense to call `Employees[j] = emp` when `emp` is already contained in `Employees` at index position `j`. Instead of replacing collection elements, you should let your EmployeeModel class implement the `INotifyPropertyChanged` interface and raise the `PropertyChanged` event when the `InBuilding` property changes. — Clemens, Apr 01 '16 at 08:31
I'm suspicious about this part `int j = Employees.IndexOf(emp); Employees[j] = emp;`. Element at `j` index doesnt' change. your code should work without exception if you use `for` loop instead of `foreach (EmployeeModel emp in Employees)`. — ASh, Apr 01 '16 at 08:33

Clemens · Accepted Answer · 2016-04-01T09:27:26.507

There is no need to replace an element in the Employees collection when you just set a property of the element.

Instead of that, your EmployeeModel class should implement the INotifyPropertyChanged interface and raise the PropertyChanged event when the InBuilding property changes:

public class EmployeeModel : INotifyPropertyChanged
{
    public event PropertyChangedEventHandler PropertyChanged;

    private bool inBuilding;
    public bool InBuilding
    {
        get { return inBuilding; }
        set
        {
            if (inBuilding != value)
            {
                inBuilding = value;
                OnPropertyChanged("InBuilding");
            }
        }
    }

    private void OnPropertyChanged(string propertyName)
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
    }

    ...
}

Now the inner loop of the update code could be reduced to this:

foreach (var emp in Employees)
{
    if (empBuild.ID == emp.ID)
    {
        emp.InBuilding = empBuild.InBuilding;
    }
}

Or you write the whole update loop like this:

foreach (var empBuild in EmployeesInBuilding)
{
    var emp = Employees.FirstOrDefault(e => e.ID == empBuild.ID);

    if (emp != null)
    {
        emp.InBuilding = empBuild.InBuilding;
    }
}

Modifying a Collection in a Foreach Loop C#

1 Answers1