Observe PropertyChanged on items in a collection

Question

I'm trying to hook into an event on INotifyPropertyChanged objects in a collection.

Every answer that I've ever seen to this question has said to handle it as follows:

void NotifyingItems_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
{
    if( e.NewItems != null )
    {
        foreach( INotifyPropertyChanged item in e.NewItems )
        {
            item.PropertyChanged += new PropertyChangedEventHandler(CollectionItemChanged);
        }
    }
    if( e.OldItems != null )
    {
        foreach( ValidationMessageCollection item in e.OldItems )
        {
            item.PropertyChanged -= CollectionItemChanged;
        }
    }
}

My problem is that this completely fails whenever a developer calls Clear() on the NotifyingItems collection. When that happens, this event handler is called with e.Action == Reset and both e.NewItems and e.OldItems equal to null (I would expect the latter to contain all items).

The problem is those items don't go away, and they aren't destroyed, they are just no longer supposed to be monitored by the current class - but since I never got the chance to unmap their PropertyChangedEventHandler - they keep calling my CollectionItemChanged handler even after they've been cleared from my NotifyingItems list. How is such a situation supposed to be handled with this 'well established' pattern?

Answer 1

Ultimate solution discovered

I've found a solution that allows the user to both capitalize on the efficiency of adding or removing many items at a time while only firing one event - and satisfy the needs of UIElements to get the Action.Reset event args while all other users would like a list of elements added and removed.

This solution involves overriding the CollectionChanged event. When we go to fire this event, we can actually look at the target of each registered handler and determine their type. Since only ICollectionView classes require NotifyCollectionChangedAction.Reset args when more than one item changes, we can single them out, and give everyone else proper event args that contain the full list of items removed or added. Below is the implementation.

public class BaseObservableCollection<T> : ObservableCollection<T>
{
    //Flag used to prevent OnCollectionChanged from firing during a bulk operation like Add(IEnumerable<T>) and Clear()
    private bool _SuppressCollectionChanged = false;

    /// Overridden so that we may manually call registered handlers and differentiate between those that do and don't require Action.Reset args.
    public override event NotifyCollectionChangedEventHandler CollectionChanged;

    public BaseObservableCollection() : base(){}
    public BaseObservableCollection(IEnumerable<T> data) : base(data){}

    #region Event Handlers
    protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
    {
        if( !_SuppressCollectionChanged )
        {
            base.OnCollectionChanged(e);
            if( CollectionChanged != null )
                CollectionChanged.Invoke(this, e);
        }
    }

    //CollectionViews raise an error when they are passed a NotifyCollectionChangedEventArgs that indicates more than
    //one element has been added or removed. They prefer to receive a "Action=Reset" notification, but this is not suitable
    //for applications in code, so we actually check the type we're notifying on and pass a customized event args.
    protected virtual void OnCollectionChangedMultiItem(NotifyCollectionChangedEventArgs e)
    {
        NotifyCollectionChangedEventHandler handlers = this.CollectionChanged;
        if( handlers != null )
            foreach( NotifyCollectionChangedEventHandler handler in handlers.GetInvocationList() )
                handler(this, !(handler.Target is ICollectionView) ? e : new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
    }
    #endregion

    #region Extended Collection Methods
    protected override void ClearItems()
    {
        if( this.Count == 0 ) return;

        List<T> removed = new List<T>(this);
        _SuppressCollectionChanged = true;
        base.ClearItems();
        _SuppressCollectionChanged = false;
        OnCollectionChangedMultiItem(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, removed));
    }

    public void Add(IEnumerable<T> toAdd)
    {
        if( this == toAdd )
            throw new Exception("Invalid operation. This would result in iterating over a collection as it is being modified.");

        _SuppressCollectionChanged = true;
        foreach( T item in toAdd )
            Add(item);
        _SuppressCollectionChanged = false;
        OnCollectionChangedMultiItem(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, new List<T>(toAdd)));
    }

    public void Remove(IEnumerable<T> toRemove)
    {
        if( this == toRemove )
            throw new Exception("Invalid operation. This would result in iterating over a collection as it is being modified.");

        _SuppressCollectionChanged = true;
        foreach( T item in toRemove )
            Remove(item);
        _SuppressCollectionChanged = false;
        OnCollectionChangedMultiItem(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, new List<T>(toRemove)));
    }
    #endregion
}

Thanks to everyone for their suggestions and links. I never would have gotten to this point without seeing all the incrementally better solutions other people came up with.

Answer 2

Edit: This solution doesn't work

This solution from the question Rachel linked to appears to be brilliant:

If I replace my NotifyingItems ObservableCollection with an inheriting class that overrides the overrideable Collection.ClearItems() method, then I can intercept the NotifyCollectionChangedEventArgs and replace it with a Remove instead of a Reset operation, and pass the list of removed items:

//Makes sure on a clear, the list of removed items is actually included.
protected override void ClearItems()
{
    if( this.Count == 0 ) return;

    List<T> removed = new List<T>(this);
    base.ClearItems();
    base.OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, removed));
}

protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
{
    //If the action is a reset (from calling base.Clear()) our overriding Clear() will call OnCollectionChanged, but properly.
    if( e.Action != NotifyCollectionChangedAction.Reset )
        base.OnCollectionChanged(e);
}

Brilliant, and nothing needs to be changed anywhere except in my own class.

---

*edit*

I loved this solution, but it doesn't work... You're not allowed to raise a NotifyCollectionChangedEventArgs that has more than one item changed unless the action is "Reset". You get the following runtime exception: Range actions are not supported. I don't know why it has to be so damn picky about this, but now this leaves no option but to remove each item one at a time... firing a new CollectionChanged event for each one. What a damn hassle.

Answer 3

Perhaps take a look at this answer

It suggests not using .Clear() and implementing a .RemoveAll() extension method that will remove the items one-by-one

public static void RemoveAll(this IList list)
{
   while (list.Count > 0)
   {
      list.RemoveAt(list.Count - 1);
   }
}

If that doesn't work for you, there are other good solutions posted in the link as well.

Answer 4

I solved this problem by making my own subclass of ObservableCollection<T> which overrides the ClearItems method. Before calling the base implementation, it raises a CollectionChanging event which I defined on my class.

CollectionChanging fires before the collection actually gets cleared, and thus you have the opportunity to subscribe to the event and unsubscribe from the events.

Example:

public event NotifyCollectionChangedEventHandler CollectionChanging;

protected override void ClearItems()
{
    if (this.Items.Count > 0)
    {
        this.OnCollectionChanging(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
    }

    base.ClearItems();
}

protected virtual void OnCollectionChanging(NotifyCollectionChangedEventArgs eventArgs)
{
    if (this.CollectionChanging != null)
    {
        this.CollectionChanging(this, eventArgs);
    }
}

Answer 5

Reset does not provide the changed items. You would need to maintain a seperate collection to clear the events if you continued to use Clear.

A easier and more memory efficient solution would be to create your own clear function and remove each item instead of calling the collection's clear.

    void ClearCollection()
    {
        while(collection.Count > 0)
        {
            // Could handle the event here...
            // collection[0].PropertyChanged -= CollectionItemChanged;
            collection.RemoveAt(collection.Count -1);
        }
    }