ObservableCollection : calling OnCollectionChanged with multiple new items

please note that I am trying to use NotifyCollectionChangedAction.Add action instead of .Reset. the latter does work, but it is not very efficient with large collections.

so i subclassed ObservableCollection:

public class SuspendableObservableCollection<T> : ObservableCollection<T>

for some reason, this code:

private List<T> _cachedItems;
...

    public void FlushCache() {
        if (_cachedItems.Count > 0) {

        foreach (var item in _cachedItems)
            Items.Add(item);

        OnCollectionChanged(new NotifyCollectionChangedEventArgs(
            NotifyCollectionChangedAction.Add, (IList<T>)_cachedItems));
        }
    }

is throwing A collection Add event refers to item that does not belong to collection

this appears to be a bug in BCL ?

I can step through and see prior to calling OnCollectionChanged that new items are added to this.Items

WOW

just made a staggering discovery. None of these approaches worked for me (flush, addrange), because the error appears to be triggered O开发者_运维问答NLY if this collection is bound to my Listview!!

TestObservableCollection<Trade> testCollection = new TestObservableCollection<Trade>();
List<Trade> testTrades = new List<Trade>();

for (int i = 0; i < 200000; i++) 
    testTrades.Add(t);

testCollection.AddRange(testTrades); // no problems here.. 
_trades.AddRange(testTrades); // this one is bound to ListView .. BOOOM!!!

In conclusion, ObservableCollection does support adding incremental lists, but a ListView doesn't. Andyp figured out a workaround to make it work with CollectionView below, but since .Refresh() is called, that is no different than just calling OnCollectionChanged( .Reset )..

you can implement AddRange() for the ObservableCollection like this as shown here:

public class RangeObservableCollection<T> : ObservableCollection<T>
{
    private bool _SuppressNotification;

    public override event NotifyCollectionChangedEventHandler CollectionChanged;

    protected virtual void OnCollectionChangedMultiItem(
        NotifyCollectionChangedEventArgs e)
    {
        NotifyCollectionChangedEventHandler handlers = this.CollectionChanged;
        if (handlers != null)
        {
            foreach (NotifyCollectionChangedEventHandler handler in 
                handlers.GetInvocationList())
            {
                if (handler.Target is CollectionView)
                    ((CollectionView)handler.Target).Refresh();
                else
                    handler(this, e);
            }
        }
    }

    protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
    {
        if (!_SuppressNotification)
        {
            base.OnCollectionChanged(e);
            if (CollectionChanged != null)
                CollectionChanged.Invoke(this, e);
        }
    }

    public void AddRange(IEnumerable<T> list)
    {
        if (list == null)
            throw new ArgumentNullException("list");

        _SuppressNotification = true;

        foreach (T item in list)
        {
            Add(item);
        }
        _SuppressNotification = false;

        OnCollectionChangedMultiItem(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, list));
    }
}

UPDATE: After binding to ListBox I was seeing an InvalidOperationException too (same message you were seeing). According to this article that's because CollectionView doesn't support range actions. Luckily the article also supplies a solution (although it feels a little "hack-ish").

UPDATE 2: Added a fix that raises the overridden CollectionChanged event in the overridden implementation of OnCollectionChanged().

Thanks for the inspiration AndyP. I had a few problems with your implementation, such as the use of CollectionView instead of ICollectionView in the test, as well as manually calling "Reset" on the elements. Elements that inherit from CollectionView might actually deal with these args in more ways than calling "this.Reset()", so it's preferable to still fire their handlers, just with the Action=Reset args that they require instead of the improved event args that include the list of items changed. Below is my (very similar) 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
}

After many iterations, we ended up with this version of ObservableRangeCollection and ReadOnlyObservableRangeCollection which is based on the code from the accepted answer, and which we didn't have to modify in the last 6 months:

public class ObservableRangeCollection<T> : ObservableCollection<T>
{
    private bool suppressNotification;

    public ObservableRangeCollection() { }

    public ObservableRangeCollection(IEnumerable<T> items)
        : base(items)
    {
    }

    public override event NotifyCollectionChangedEventHandler CollectionChanged;

    protected virtual void OnCollectionChangedMultiItem(
        NotifyCollectionChangedEventArgs e)
    {
        var handlers = CollectionChanged;
        if (handlers == null) return;

        foreach (NotifyCollectionChangedEventHandler handler in handlers.GetInvocationList())
        {
            if (handler.Target is ReadOnlyObservableCollection<T>
                && !(handler.Target is ReadOnlyObservableRangeCollection<T>))
            {
                throw new NotSupportedException(
                    "ObservableRangeCollection is wrapped in ReadOnlyObservableCollection which might be bound to ItemsControl " +
                    "which is internally using ListCollectionView which does not support range actions.\n" +
                    "Instead of ReadOnlyObservableCollection, use ReadOnlyObservableRangeCollection");
            }
            var collectionView = handler.Target as ICollectionView;
            if (collectionView != null)
            {
                collectionView.Refresh();
            }
            else
            {
                handler(this, e);
            }
        }
    }

    protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
    {
        if (suppressNotification) return;

        base.OnCollectionChanged(e);
        if (CollectionChanged != null)
        {
            CollectionChanged.Invoke(this, e);
        }
    }

    public void AddRange(IEnumerable<T> items)
    {
        if (items == null) return;

        suppressNotification = true;

        var itemList = items.ToList();

        foreach (var item in itemList)
        {
            Add(item);
        }
        suppressNotification = false;

        if (itemList.Any())
        {
            OnCollectionChangedMultiItem(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, itemList));
        }
    }

    public void AddRange(params T[] items)
    {
        AddRange((IEnumerable<T>)items);
    }

    public void ReplaceWithRange(IEnumerable<T> items)
    {
        Items.Clear();
        OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
        AddRange(items);
    }

    public void RemoveRange(IEnumerable<T> items)
    {
        suppressNotification = true;

        var removableItems = items.Where(x => Items.Contains(x)).ToList();

        foreach (var item in removableItems)
        {
            Remove(item);
        }

        suppressNotification = false;

        if (removableItems.Any())
        {
            OnCollectionChangedMultiItem(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, removableItems));
        }
    }
}

public class ReadOnlyObservableRangeCollection<T> : ReadOnlyObservableCollection<T>
{
    public ReadOnlyObservableRangeCollection(ObservableCollection<T> list)
        : base(list)
    {            
    }

    protected override event NotifyCollectionChangedEventHandler CollectionChanged;

    protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
    {
        var handlers = CollectionChanged;
        if (handlers == null) return;

        foreach (NotifyCollectionChangedEventHandler handler in handlers.GetInvocationList())
        {
            var collectionView = handler.Target as ICollectionView;
            if (collectionView != null)
            {
                collectionView.Refresh();
            }
            else
            {
                handler(this, e);
            }
        }
    }
}

We basically replaced all usages of ObservableCollection in our app by ObservableRangeCollection, and it works like a charm.

I believe you need to cast it to an IList:

base.OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, (IList)_cachedItems));