possible improvement to BindableCollection

Mar 3, 2011 at 6:29 PM

Some collection views (such as CollectionView and ListCollectionView) will throw a NotSupportedException("Range actions are not supported.") in some situations, so I modified BindableCollection.OnCollectionChanged to trap this exception and try to call Refresh() instead. I don't know if this is the best way to handle this, but it is working for me.

 

private NotifyCollectionChangedEventHandler _collectionChanged;
public override event NotifyCollectionChangedEventHandler CollectionChanged
{
	add { _collectionChanged += value; }
	remove { _collectionChanged -= value; }
}

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

	var collectionChanged = _collectionChanged;
	if (collectionChanged == null)
		return;

	using (BlockReentrancy())
	{
		foreach (NotifyCollectionChangedEventHandler handler in collectionChanged.GetInvocationList())
		{
			try
			{
				handler(this, e);
			}
			catch (NotSupportedException ex)
			{
				bool exceptionHandled = false;
				if (typeof(ICollectionView).IsAssignableFrom(ex.TargetSite.DeclaringType)
					&& handler.Target is ICollectionView)
				{
					/// Some implementations of ICollectionView do not support range operations, so just tell it to refresh
					try
					{
						((ICollectionView)handler.Target).Refresh();
						exceptionHandled = true;
					}
					catch { }
				}
				if (!exceptionHandled) 
					throw;
			}
		}
	}
}

Mar 4, 2011 at 9:19 AM

I think it should throw an exception because the items that a developer may be trying to add via an addrange may not exist in the collection just because it's refreshed. 

Mar 4, 2011 at 9:33 AM

I think that hiding an exception is not the way to go.

I hate the fact that the code base throws an exception for range operations (and still cannot understand why the NotifyCollectionChangedAction.Reset does not provide the removed items too), but preventing the developer to know that he/she is going against the standard behaviour of the framework can result in strange behaviours.

Moreover the proposed change acts the same way as implementing collections that issue a Reset operation on ranged operations (this is the approach used by BindableCollection<T>, by the way). In the worst case scenario, the above code will throw an exception, swallow it and issue a reset. Invoking a Reset action will simply trigger the reset, without the additional cost of the exception handling.

Mar 4, 2011 at 6:18 PM

There's the problem! I didn't realize that one of our developers had changed the actions from Reset to Add and Remove for the range operations. But I understand why it was done. We want to be able to automatically subscribe/unsubscribe to the PropertyChanged event of items as they are added to/removed from the collection. Reset does not seem to give us an opportunity to do so.

I think I will either add some optional delegates to BindableCollection or a subclass of it that will be called on each item as it is added/removed.

Thanks