views:

3573

answers:

10

I have something here that is really catching me off guard.

I have an ObservableCollection of T that is filled with items. I also have an event handler attached to the CollectionChanged event.

When you Clear the collection it causes an CollectionChanged event with e.Action set to NotifyCollectionChangedAction.Reset. Ok, that's normal. But what is weird is that neither e.OldItems or e.NewItems has anything in it. I would expect e.OldItems to be filled with all items that were removed from the collection.

Has anyone else seen this? And if so, how have they gotten around it?

Some background: I am using the CollectionChanged event to attach and detach from another event and thus if I don't get any items in e.OldItems ... I won't be able to detach from that event.


CLARIFICATION: I do know that the documentation doesn't outright state that it has to behave this way. But for every other action, it is notifying me of what it has done. So, my assumption is that it would tell me ... in the case of Clear/Reset as well.


Below is the sample code if you wish to reproduce it yourself. First off the xaml:

<Window
    x:Class="ObservableCollection.Window1"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    Title="Window1"
    Height="300"
    Width="300"
>
    <StackPanel>
        <Button x:Name="addButton" Content="Add" Width="100" Height="25" Margin="10" Click="addButton_Click"/>
        <Button x:Name="moveButton" Content="Move" Width="100" Height="25" Margin="10" Click="moveButton_Click"/>
        <Button x:Name="removeButton" Content="Remove" Width="100" Height="25" Margin="10" Click="removeButton_Click"/>
        <Button x:Name="replaceButton" Content="Replace" Width="100" Height="25" Margin="10" Click="replaceButton_Click"/>
        <Button x:Name="resetButton" Content="Reset" Width="100" Height="25" Margin="10" Click="resetButton_Click"/>
    </StackPanel>
</Window>

Next, the code behind:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Data;
using System.Windows.Documents;
using System.Windows.Input;
using System.Windows.Media;
using System.Windows.Media.Imaging;
using System.Windows.Navigation;
using System.Windows.Shapes;
using System.Collections.ObjectModel;

namespace ObservableCollection
{
    /// <summary>
    /// Interaction logic for Window1.xaml
    /// </summary>
    public partial class Window1 : Window
    {
        public Window1()
        {
            InitializeComponent();
            _integerObservableCollection.CollectionChanged += new System.Collections.Specialized.NotifyCollectionChangedEventHandler(_integerObservableCollection_CollectionChanged);
        }

        private void _integerObservableCollection_CollectionChanged(object sender, System.Collections.Specialized.NotifyCollectionChangedEventArgs e)
        {
            switch (e.Action)
            {
                case System.Collections.Specialized.NotifyCollectionChangedAction.Add:
                    break;
                case System.Collections.Specialized.NotifyCollectionChangedAction.Move:
                    break;
                case System.Collections.Specialized.NotifyCollectionChangedAction.Remove:
                    break;
                case System.Collections.Specialized.NotifyCollectionChangedAction.Replace:
                    break;
                case System.Collections.Specialized.NotifyCollectionChangedAction.Reset:
                    break;
                default:
                    break;
            }
        }

        private void addButton_Click(object sender, RoutedEventArgs e)
        {
            _integerObservableCollection.Add(25);
        }

        private void moveButton_Click(object sender, RoutedEventArgs e)
        {
            _integerObservableCollection.Move(0, 19);
        }

        private void removeButton_Click(object sender, RoutedEventArgs e)
        {
            _integerObservableCollection.RemoveAt(0);
        }

        private void replaceButton_Click(object sender, RoutedEventArgs e)
        {
            _integerObservableCollection[0] = 50;
        }

        private void resetButton_Click(object sender, RoutedEventArgs e)
        {
            _integerObservableCollection.Clear();
        }

        private ObservableCollection<int> _integerObservableCollection = new ObservableCollection<int> { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19 };
    }
}
+2  A: 

Looking at the NotifyCollectionChangedEventArgs, it appears that OldItems only contains items changed as a result of Replace, Remove, or Move action. It doesn't indicate that it will contain anything on Clear. I suspect that Clear fires the event, but does not registered the removed items and does not invoke the Remove code at all.

tvanfosson
I saw that too, but I do not like it. It seems like a gaping hole to me.
cplotts
It doesn't invoke the remove code because it doesn't need to. Reset means "something dramatic has happened, you need to start again". A clear operation is one example of this, but there are others
Orion Edwards
+2  A: 

This is how ObservableCollection works, you can work around this by keeping your own list outside of the ObservableCollection (adding to the list when action is Add, remove when action is Remove etc.) then you can get all the removed items (or added items) when action is Reset by comparing your list with the ObservableCollection.

Another option is to create your own class that implements IList and INotifyCollectionChanged, then you can attach and detach events from within that class (or set OldItems on Clear if you like) - it's really not difficult, but it is a lot of typing.

Nir
I considered keeping track of another list as well as you suggest first, but it seems like a lot of unnecessary work. Your second suggestion is very close to what I ended up going with ... which I will post as an answer.
cplotts
+2  A: 

We had the same issue here. The Reset action in CollectionChanged does not include the OldItems. We had a workaround: we used instead the following extension method:

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

We ended up not supporting the Clear() function, and throwing a NotSupportedException in CollectionChanged event for Reset actions. The RemoveAll will trigger a Remove action in CollectionChanged event, with the proper OldItems.

decasteljau
Good idea. I don't like not supporting Clear as that is the method (in my experience) most people use ... but at least you are warning the user with an exception.
cplotts
I agree, this is not the ideal solution, but we found it to be the best acceptable workaround.
decasteljau
You're not supposed to use the old items! What you're supposed to do is dump whatever data you have on the list, and re-scan it as if it were a new list!
Orion Edwards
The problem, Orion, with your suggestion ... is the use case that prompted this question. What happens when I have items in the list that I want to detach an event from? I can't just dump the data on the list ... it would result in memory leaks/pressure.
cplotts
Now, I do understand that you are arguing about the semantics of Reset and you bring up some interesting points. But, I would love to see some documentation on what you're suggesting.
cplotts
Another point to make here ... is that by the time you get the CollectionChanged event with the Action of Reset (from calling Clear) ... the collection is ALREADY empty.Thus why you can't use it to detach from events. So, again, I can't simply just *dump* my data. It's already been dumped for me.
cplotts
+1  A: 

Ok, even though I still wish that ObservableCollection behaved as I wished ... the code below is what I ended up doing. Basically, I created a new collection of T called TrulyObservableCollection and overrided the ClearItems method which I then used to raise a Clearing event.

In the code that uses this TrulyObservableCollection, I use this Clearing event to loop through the items that are still in the collection at that point to do the detach on the event that I was wishing to detach from.

Hope this approach helps someone else as well.

public class TrulyObservableCollection<T> : ObservableCollection<T>
{
    public event EventHandler<EventArgs> Clearing;
    protected virtual void OnClearing(EventArgs e)
    {
        if (Clearing != null)
            Clearing(this, e);
    }

    protected override void ClearItems()
    {
        OnClearing(EventArgs.Empty);
        base.ClearItems();
    }
}
cplotts
You need to rename your class to `BrokenObservableCollection`, not `TrulyObservableCollection` - you're misunderstanding what the reset action means.
Orion Edwards
@Orion Edwards: I disagree. See my comment to your answer.
cplotts
@Orion Edwards: Oh, wait, I see, you're being funny. But then I should really call it: `ActuallyUsefulObservableCollection`. :)
cplotts
Lol great name. I agree this is a serious oversight in the design.
chaiguy
+1  A: 

I tackled this one in a slightly different manner as I wanted to register to one event and handle all additions and removals in the event handler. I started off overriding the collection changed event and redirecting reset actions to removal actions with a list of items. This all went wrong as I was using the observable collection as an items source for a collection view and got "Range actions not supported".

I finally created a new event called CollectionChangedRange which acts in the manner I expected the inbuilt version to act.

I can't imagine why this limitation would be allowed and hope that this post at least stops others from going down the dead end that I did.

/// <summary>
/// An observable collection with support for addrange and clear
/// </summary>
/// <typeparam name="T"></typeparam>
[Serializable]
[TypeConverter(typeof(ExpandableObjectConverter))]
public class ObservableCollectionRange<T> : ObservableCollection<T>
{
    private bool _addingRange;

    [field: NonSerialized]
    public event NotifyCollectionChangedEventHandler CollectionChangedRange;

    protected virtual void OnCollectionChangedRange(NotifyCollectionChangedEventArgs e)
    {
        if ((CollectionChangedRange == null) || _addingRange) return;
        using (BlockReentrancy())
        {
            CollectionChangedRange(this, e);
        }
    }

    public void AddRange(IEnumerable<T> collection)
    {
        CheckReentrancy();
        var newItems = new List<T>();
        if ((collection == null) || (Items == null)) return;
        using (var enumerator = collection.GetEnumerator())
        {
            while (enumerator.MoveNext())
            {
                _addingRange = true;
                Add(enumerator.Current);
                _addingRange = false;
                newItems.Add(enumerator.Current);
            }
        }
        OnCollectionChangedRange(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, newItems));
    }

    protected override void ClearItems()
    {
        CheckReentrancy();
        var oldItems = new List<T>(this);
        base.ClearItems();
        OnCollectionChangedRange(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, oldItems));
    }

    protected override void InsertItem(int index, T item)
    {
        CheckReentrancy();
        base.InsertItem(index, item);
        OnCollectionChangedRange(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, index));
    }

    protected override void MoveItem(int oldIndex, int newIndex)
    {
        CheckReentrancy();
        var item = base[oldIndex];
        base.MoveItem(oldIndex, newIndex);
        OnCollectionChangedRange(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Move, item, newIndex, oldIndex));
    }

    protected override void RemoveItem(int index)
    {
        CheckReentrancy();
        var item = base[index];
        base.RemoveItem(index);
        OnCollectionChangedRange(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, item, index));
    }

    protected override void SetItem(int index, T item)
    {
        CheckReentrancy();
        var oldItem = base[index];
        base.SetItem(index, item);
        OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, oldItem, item, index));
    }
}

/// <summary>
/// A read only observable collection with support for addrange and clear
/// </summary>
/// <typeparam name="T"></typeparam>
[Serializable]
[TypeConverter(typeof(ExpandableObjectConverter))]
public class ReadOnlyObservableCollectionRange<T> : ReadOnlyObservableCollection<T>
{
    [field: NonSerialized]
    public event NotifyCollectionChangedEventHandler CollectionChangedRange;

    public ReadOnlyObservableCollectionRange(ObservableCollectionRange<T> list) : base(list)
    {
        list.CollectionChangedRange += HandleCollectionChangedRange;
    }

    private void HandleCollectionChangedRange(object sender, NotifyCollectionChangedEventArgs e)
    {
        OnCollectionChangedRange(e);
    }

    protected virtual void OnCollectionChangedRange(NotifyCollectionChangedEventArgs args)
    {
        if (CollectionChangedRange != null)
        {
            CollectionChangedRange(this, args);
        }
    }

}
Jim Hough
Interesting approach. Thanks for posting it. If I ever run into problems with my own approach, I think I will revisit yours.
cplotts
A: 

I was just going through some of the charting code in the Silverlight and WPF toolkits and noticed that they also solved this problem (in a kind of similar way) ... and I thought I would go ahead and post their solution.

Basically, they also created a derived ObservableCollection and overrode ClearItems, calling Remove on each item being cleared.

Here is the code:

/// <summary>
/// An observable collection that cannot be reset.  When clear is called
/// items are removed individually, giving listeners the chance to detect
/// each remove event and perform operations such as unhooking event 
/// handlers.
/// </summary>
/// <typeparam name="T">The type of item in the collection.</typeparam>
public class NoResetObservableCollection<T> : ObservableCollection<T>
{
    public NoResetObservableCollection()
    {
    }

    /// <summary>
    /// Clears all items in the collection by removing them individually.
    /// </summary>
    protected override void ClearItems()
    {
        IList<T> items = new List<T>(this);
        foreach (T item in items)
        {
            Remove(item);
        }
    }
}
cplotts
I just want to point out that I don't like this approach as much as the one I marked as an answer ... since you get a NotifyCollectionChanged event (with a Remove action) ... for EACH item being removed.
cplotts
+1  A: 

Well I decided to get dirty with it. MS put a LOT of work into making the NotifyCollectionChangedEventArgs always contain no data when calling a reset. I'm assuming this was a performance/memory decision. If you are resetting a collection with 100000 elements I'm assuming they didn't want to duplicate all that. But seeing as My collections never have more then 100 elements I don't see a problem with it.

Anyway I created an inherated class with the following method

protected override void ClearItems() {
        CheckReentrancy();
        List<TItem> oldItems = new List<TItem>(Items);

        Items.Clear();

        OnPropertyChanged(new PropertyChangedEventArgs("Count"));
        OnPropertyChanged(new PropertyChangedEventArgs("Item[]"));

        NotifyCollectionChangedEventArgs e = new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset);

        FieldInfo field = e.GetType().GetField("_oldItems", BindingFlags.Instance | BindingFlags.NonPublic);
        field.SetValue(e, oldItems);

        OnCollectionChanged(e);
    }
HaxElit
+1 for hack value.
David Schmitt
This is cool, but probably wouldn't work in anything but a full-trust environment. Reflecting over private fields requires full-trust, right?
Paul
Why would you do this? There are other things that can cause the Reset action to fire - just because you've disabled the clear method doesn't mean it's gone away (or that it should)
Orion Edwards
Interesting approach, but reflection can be slow.
cplotts
A: 

For the scenario of attaching and detaching event handlers to the elements of the ObservableCollection there is also a "client-side" solution. In the event handling code you can check if the sender is in the ObservableCollection using the Contains method. Pro: you can work with any existing ObservableCollection. Cons: the Contains method runs with O(n) where n is the number of elements in the ObservableCollection. So this is a solution for small ObservableCollections.

Another "client-side" solution is to use an event handler in the middle. Just register all events to the event handler in the middle. This event handler in turn notifies the real event handler trough a callback or an event. If a Reset action occurs remove the callback or event create a new event handler in the middle and forget about the old one. This approach also works for big ObservableCollections. I used this for the PropertyChanged event (see code below).

    /// <summary>
    /// Helper class that allows to "detach" all current Eventhandlers by setting
    /// DelegateHandler to null.
    /// </summary>
    public class PropertyChangedDelegator
    {
        /// <summary>
        /// Callback to the real event handling code.
        /// </summary>
        public PropertyChangedEventHandler DelegateHandler;
        /// <summary>
        /// Eventhandler that is registered by the elements.
        /// </summary>
        /// <param name="sender">the element that has been changed.</param>
        /// <param name="e">the event arguments</param>
        public void PropertyChangedHandler(Object sender, PropertyChangedEventArgs e)
        {
            if (DelegateHandler != null)
            {
                DelegateHandler(sender, e);
            }
            else
            {
                INotifyPropertyChanged s = sender as INotifyPropertyChanged;
                if (s != null)
                    s.PropertyChanged -= PropertyChangedHandler;
            }   
        }
    }
Chris
@Chris: I believe with your first approach, I would need another list to track the items ... because once you get the CollectionChanged event with the Reset action ... the collection is already empty.I don't quite follow your second suggestion. I would love a simple test harness illustrating it, but for adding, removing, and clearing the ObservableCollection.If you build an example, you can email me at my first name followed by my last name at gmail.com.
cplotts
+1  A: 

It's not supposed to include old items. Reset doesn't mean that the list has been cleared!

It means that some dramatic thing has taken place, and the cost of working out the add/removes would most likely exceed the cost of just re-scanning the list from scratch... so that's what you should do.

MSDN suggests an example of the entire collection being re-sorted as a candidate for reset.

To reiterate. Reset doesn't mean clear, it means Your assumptions about the list are now invalid. Treat it as if it's an entirely new list. Clear happens to be one instance of this, but there can and will be others.

Orion Edwards
I'm going to respectfully disagree on this basis. If you look at the documentation it states: Represents a dynamic data collection that provides notifications when items get added, removed, or when the whole list is refreshed (see http://msdn.microsoft.com/en-us/library/ms668613(v=VS.100).aspx)
cplotts
I just want to point out again (as I did in the question), that I know that the documentation doesn't OUTRIGHT say that calling Clear should notify you what items are being removed ... but in this case, it seems to me that the collection is NOT really observing changes.
cplotts
It is notifying you when items are removed. It's just not telling you which items, because it's deemed to be more trouble than it's worth. If you want a slower-but-more-explicit collection, then more power to you, but don't blame the `ObservableCollection` because you don't understand it :-)
Orion Edwards
A: 

The ObservableCollection as well as the INotifyCollectionChanged interface are clearly written with a specific use in mind: UI building and its specific performance characteristics.

When you want notifications of collection changes then you are generally only interested in Add en Remove events.

I use the following interface:

using System;
using System.Collections.Generic;

/// <summary>
/// Notifies listeners of the following situations:
/// <list type="bullet">
/// <item>Elements have been added.</item>
/// <item>Elements are about to be removed.</item>
/// </list>
/// </summary>
/// <typeparam name="T">The type of elements in the collection.</typeparam>
interface INotifyCollection<T>
{
    /// <summary>
    /// Occurs when elements have been added.
    /// </summary>
    event EventHandler<NotifyCollectionEventArgs<T>> Added;

    /// <summary>
    /// Occurs when elements are about to be removed.
    /// </summary>
    event EventHandler<NotifyCollectionEventArgs<T>> Removing;
}

/// <summary>
/// Provides data for the NotifyCollection event.
/// </summary>
/// <typeparam name="T">The type of elements in the collection.</typeparam>
public class NotifyCollectionEventArgs<T> : EventArgs
{
    /// <summary>
    /// Gets or sets the elements.
    /// </summary>
    /// <value>The elements.</value>
    public IEnumerable<T> Items
    {
        get;
        set;
    }
}

I've also written my own overload of Collection where:

  • ClearItems raises Removing
  • InsertItem raises Added
  • RemoveItem raises Removing
  • SetItem raises Removing and Added

Of course, AddRange can be added as well.

Rick Beerendonk