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?

possible duplicate of [When Clearing an ObservableCollection, There are No Items in e.OldItems](http://stackoverflow.com/questions/224155/when-clearing-an-observablecollection-there-are-no-items-in-e-olditems) — Rachel, Feb 22 '12 at 16:58

score 5 · Answer 1 · edited May 23 '17 at 12:09

5

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.

edited May 23 '17 at 12:09

Community

1
1

answered Feb 22 '12 at 16:42

Rachel

130,264
66
304
490

Thanks, that actually looks like an exact duplicate of this question, just better phrased. – Alain Feb 22 '12 at 16:55
1

I see you've actually encountered this problem yourself. [Link](http://stackoverflow.com/questions/7449196/how-can-i-raise-a-collectionchanged-event-on-an-observablecollection-and-pass-i) Did you ever find a way to deal with this bulk clear without having to fire hundreds of property changed events? (IE, raise a "Clear" event for UIElements, and a Remove event for everything else?) – Alain Feb 22 '12 at 20:05
1

@Alain I never did. Instead, when the performance of `.AddRange()` or `.RemoveRange()` took too long, I just re-created the collection entirely. Usually I had something in the `set` method of the collection to unhook all the event handlers of the old collection as well before hooking up the new one. It's definitely not an ideal solution, but it worked. – Rachel Feb 22 '12 at 20:39

score 2 · Answer 2 · edited May 23 '17 at 12:01

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.

I came up for a workaround to the above runtime exception, which was being thrown by the CollectionView class (which all item listing UIElements use.) Solution is posted below: http://stackoverflow.com/a/9416568/529618 — Alain, Feb 23 '12 at 16:00

score 2 · Accepted Answer · answered Feb 23 '12 at 15:59

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.

Thank you for your solution Alain. However I found a small bug. In the methods "Add" and "Remove", you iterate twice the IEnumerable in parameters. So for instance if that IEnumerable would create objects, they would be created twice. To simply cache it before would do the trick, like this: var toAddList = toAdd as IList ?? toAdd.ToList(); And anyway, you are creating a list out of the enumerable at the end. — FrankyB, Mar 11 '14 at 12:53
@FrankyB You are correct. This was in my early days before ReSharper showed me the error of my ways :) — Alain, Mar 11 '14 at 14:19

score 1 · Answer 4 · answered Feb 22 '12 at 16:45

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);
    }
}

This is a valid solution, although I am striving for one that doesn't require the other developers to "always remember to handle this new Event I invented or it won't work." These sorts of rules that can't be enforced at compile time never work out in practice in projects with more than one developer. — Alain, Feb 22 '12 at 17:23
Well, you could always make your own collection type along the lines of what I've provided above, which internally takes care of unsubscribing when an element is removed or the collection is cleared — RobSiklos, Feb 22 '12 at 18:00

score 0 · Answer 5 · answered Feb 22 '12 at 16:43

0

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);
        }
    }

answered Feb 22 '12 at 16:43

JeremyK

1,075
1
22
45

My only problem with this solution is, as hinted in the question, this class and collection is used by other developers, and there's nothing about this code that allows me to force other developers not to use "Clear()" on the collection - the method is there and they love it. If one ever did, it would manifest itself as an extremely hard to diagnose runtime bug. – Alain Feb 22 '12 at 17:21
Creating a new inherited class and overriding the functions is really your only solution. But you have already concluded that, so good luck. – JeremyK Feb 22 '12 at 18:54

Observe PropertyChanged on items in a collection

5 Answers5