Search code examples
c#wpf

Binding to a class extending ObservableCollection breaks notifications


In my application I have a class that extends ObservableCollection for a specific type in order to simplify adding/removing items from it:

public class ObservableCollectionICadObjectInfo : ObservableCollection<ICadObjectInfo>
{
    public void AddUnique(ICadObjectInfo value)
    {
        List<ICadObjectInfo> matches = Items.Where(i => i.FullName == value.FullName).ToList();
        if (matches.Count == 0)
        {
            Items.Add(value);
        }
    }

    public void AddRangeUnique(List<ICadObjectInfo> values)
    {
        foreach (ICadObjectInfo value in values)
        {
            AddUnique(value);
        }
    }

    public void RemoveItem(ICadObjectInfo item)
    {
        if (item == null) { return; }
        ICadObjectInfo match = Items.Where(i => i.FullName == item.FullName).FirstOrDefault();
        if (match is not null)
        {
            Items.Remove(match);
        }
    }
}

An instance of this class is used in the following ModelView that serves as a DataContext for a ListBox binding:

public class CadObjectSelectorViewModel : ObservableClass
{
    private ObservableCollectionICadObjectInfo _cadObjects;
    private ICadObjectInfo _selectedObject;
    public ObservableCollectionICadObjectInfo CadObjects { get { return _cadObjects; } set { _cadObjects = value; OnPropertyChanged(nameof(CadObjects)); }  }
    public ICadObjectInfo SelectedObject { get { return _selectedObject; } set { _selectedObject = value; OnPropertyChanged(nameof(SelectedObject)); } }

    public CadObjectSelectorViewModel(ObservableCollectionICadObjectInfo cadObjects)
    {
        this.CadObjects = cadObjects;
    }
}

<ListBox x:Name="CadObjectsListBox" ItemsSource="{Binding Path=CadObjects}" SelectedItem="{Binding Path=SelectedObject}"/>

Now, if I update the items in the list (clear them, or add new items), ListBox.Items updates, but for whatever reason the ListBox itself does not, and still shows previous data:

enter image description here

Weirder still, if I clear the list, initially nothing happens, but if I leave the application to run idle for 10-40 seconds (during which it is responsive), the ListBox finally updates. Sometimes it happens sooner, sometimes later. It's almost like some GC cycle nudges the ListBox and makes it realize it is empty. Adding new items to this ListBox does not make it update no matter how long I wait though.

And if I do not use ObservableCollectionICadObjectInfo in my CadObjectSelectorViewModel, but instead declare CadObjects as ObservableCollection<ICadObjectInfo>, the issue completely disappears, everything updates immediately.

Why does using this extended ObservableCollection class cause this issue? I read through the documentation and did a proper search before asking this question, but I can't seem to find anything relevant.


Solution

  • Don't call your updates through the inner collection's update functions, call them through the ObservableCollection's:

    public void AddUnique(ICadObjectInfo value)
    {
        List<ICadObjectInfo> matches = Items.Where(i => i.FullName == value.FullName).ToList();
        if (matches.Count == 0)
        {
            Add(value);   // <----
        }
    }
    
    public void RemoveItem(ICadObjectInfo item)
    {
        if (item == null) { return; }
        ICadObjectInfo match = Items.Where(i => i.FullName == item.FullName).FirstOrDefault();
        if (match is not null)
        {
            Remove(match);  // <----
        }
    }
    

    Though just to be clear, your approach is absolutely horrendous. The fact that you managed to break something as basic as ObservableCollection should be ringing very loud alarm bells in your mind, or at the very least in your managers'. I would strongly reconsider this mess and try to understand what you're trying to achieve here.

    If it's a collection that doesn't have duplicates, you failed because you can just directly call Add, thus breaking that promise. If it's a collection with helper functions to only add if elements aren't present, you have extension methods for that. If it's a collection that's simply used to display the data without repetitions, well, don't get the repetitions in the first place. If you can't figure that out, then use either a CollectionView, or a converter.

    No matter how you cut it, the approach in your question has no place other than to confuse people and break code.