ReactiveCollection CreateDerivedCollection needs cleanup/dispose action for remove/clear items

222 views
Skip to first unread message

Jonny Garrett

unread,
May 20, 2011, 7:56:08 AM5/20/11
to ReactiveUI mailing list
When clear is called on reactive collection it doesnt call
ItemsRemoved on each item so there is no easy way to perform an action
on each item before removal. If items of a ReactiveCollection need to
be disposed or cleaned up in some way it would be cool when creating a
derived collection we could supply a Dispose or Cleanup action for
when removing and clearing items. This would allow item viewmodels to
be disposed efectively. This would complete the circle of wrapping/
unwrapping models to viewmodels when using the reactivecollection.

Jonny Garrett

unread,
May 20, 2011, 4:56:03 PM5/20/11
to ReactiveUI mailing list
Heres what I put together that does the job. Be nice to see this make
into the lib

public static ReactiveCollection<TNew> CreateDerivedCollection<T,
TNew>(
this ObservableCollection<T> This,
Func<T, TNew> selector, Action<TNew> cleaner = null)
{
Contract.Requires(selector != null);
#if !IOS // Contract.Result is borked in Mono

Contract.Ensures(Contract.Result<ReactiveCollection<TNew>>().Count ==
This.Count);
#endif
var ret = new
ReactiveCollection<TNew>(This.Select(selector));

var coll_changed = new
Subject<NotifyCollectionChangedEventArgs>(RxApp.DeferredScheduler);
This.CollectionChanged += (o, e) =>
coll_changed.OnNext(e);

/* XXX: Ditto as from above
var coll_changed =
Observable.FromEvent<NotifyCollectionChangedEventHandler,
NotifyCollectionChangedEventArgs>(
x => This.CollectionChanged += x, x =>
This.CollectionChanged -= x);
*/

coll_changed.Subscribe(x =>
{
switch (x.Action)
{
case NotifyCollectionChangedAction.Add:
case NotifyCollectionChangedAction.Remove:
case NotifyCollectionChangedAction.Replace:
// NB: SL4 fills in OldStartingIndex with -1
on Replace :-/
int old_index = (x.Action ==
NotifyCollectionChangedAction.Replace ?
x.NewStartingIndex : x.OldStartingIndex);

if (x.OldItems != null)
{
foreach (object _ in x.OldItems)
{
if (cleaner != null)
cleaner(ret[old_index]);

ret.RemoveAt(old_index);
}
}
if (x.NewItems != null)
{
foreach (T item in x.NewItems.Cast<T>())
{
ret.Insert(x.NewStartingIndex,
selector(item));
}
}
break;
case NotifyCollectionChangedAction.Reset:
if (cleaner != null)
ret.Run(cleaner);
ret.Clear();
break;
default:
break;
}
});

return ret;
}.

Paul Betts

unread,
May 25, 2011, 1:53:42 AM5/25/11
to reacti...@googlegroups.com
This is consistent with how ObservableCollection works, it also
doesn't signal items removed when Reset() is called - I could consider
adding in a "CollectionReset" IObservable though. Can you file this as
a Github bug so I don't forget?

One thing to keep in mind is, that you generally don't have to Dispose
the IDisposable returned by Subscribe unless you explicitly want to
terminate a subscription early. Objects that go out of scope and are
GC'ed will disconnect themselves automagically.

--
Paul Betts <pa...@paulbetts.org>

Daniel Harman

unread,
May 27, 2011, 11:48:10 AM5/27/11
to ReactiveUI mailing list
I ran into this problem today using a 3rd party grid control who's
selected items I had bound to a ReactiveCollection. Unfortunately they
always call reset and then add for the selected rows rather than
remove/add. So I had to do create this:

/// <summary>
/// Extends reactive collections so that .ClearItems triggers remove
for the items.
/// </summary>
public class ReactiveCollectionEx<T> :
ReactiveUI.ReactiveCollection<T>
{
public ReactiveCollectionEx() : base()
{}

public ReactiveCollectionEx(IEnumerable<T> list)
: base(list)
{}

protected override void ClearItems()
{
for (int i = this.Count - 1; i >= 0; i--)
{
this.RemoveAt(i);
}
}
}

I appreciate it is inconsistent with ObservableCollection, but
sometimes needs must! The only other option would have been to extend
with the CollectionReset IObservable, but I was using this for
filtering, and the logic there likes Add/Remove (and given the small
number of rows selected performance is fine).

Dan

On May 25, 6:53 am, Paul Betts <paul.be...@gmail.com> wrote:
> This is consistent with how ObservableCollection works, it also
> doesn't signal items removed when Reset() is called - I could consider
> adding in a "CollectionReset" IObservable though. Can you file this as
> a Github bug so I don't forget?
>
> One thing to keep in mind is, that you generally don't have to Dispose
> the IDisposable returned by Subscribe unless you explicitly want to
> terminate a subscription early. Objects that go out of scope and are
> GC'ed will disconnect themselves automagically.
>
> --
> Paul Betts <p...@paulbetts.org>

Jonny Garrett

unread,
May 31, 2011, 1:02:12 PM5/31/11
to ReactiveUI mailing list
It may not be consistent with ObservableCollection but it didnt have
observables either. :) Isnt the goal here to make things better and
add the stuff that .NET forgot. I have use cases where I need to hook
into just before a collection is reset as well after it, so I'd
apreciate BeforeCollectionReset and CollectionReset observables. This
is just added functionaltiy that improves the collection and would
have no impact on existing code.

Jonny Garrett

unread,
May 31, 2011, 1:04:15 PM5/31/11
to ReactiveUI mailing list
Oh and the resason for the cleaner or deselector function is because I
cant rely on the GC to cleanup things that have nasty side efects. I
need to dispose them explicitly.
Reply all
Reply to author
Forward
0 new messages