DerivedCollection onRemove clean up

Author: TheGrandUser

ReactiveUI.Tests/ReactiveCollectionTest.cs

@@ -967,8 +967,7 @@ public void DerviedCollectionShouldHandleItemsRemoved()
            var fixture = new ReactiveList<TestFixture>(
                input.Select(x => new TestFixture() { IsOnlyOneWord = x }));
 
-           var output = fixture.CreateDerivedCollection(new Func<TestFixture, IDisposable>(x => Disposable.Create(() => disposed.Add(x))),
-              onRemoved: item => item.Dispose());
+           var output = fixture.CreateDerivedCollection(x => Disposable.Create(() => disposed.Add(x)), item => item.Dispose());
 
            fixture.Add(new TestFixture() { IsOnlyOneWord = "Hello" });
            Assert.Equal(5, output.Count);
@@ -1697,8 +1696,7 @@ ReactiveList<TextModel> makeAsyncCollection(int maxSize)
         }
 
         [Fact]
-        public void TestDelayNotifications() 
-        {
+        public void TestDelayNotifications()  {
             var maxSize = 10;
             var data = makeAsyncCollection(maxSize);
 
@@ -1708,7 +1706,6 @@ public void TestDelayNotifications()
 
             var derivedList = list.CreateDerivedCollection(
                 m => m.Value, m => m.HasData, (a, b) => a.Text.CompareTo(b.Text),
-                null,
                 Observable.Never(4) /*list.ShouldReset*/, 
                 scheduler: RxApp.MainThreadScheduler);
 

ReactiveUI.Winforms/Winforms/ReactiveDerivedBindingListMixins.cs

@@ -105,6 +105,8 @@ public static class ObservableCollectionMixin
         /// </summary>
         /// <param name="selector">A Select function that will be run on each
         /// item.</param>
+        /// <param name="onRemoved">An action that is called on each item when
+        /// it is removed.</param>
         /// <param name="filter">A filter to determine whether to exclude items 
         /// in the derived collection.</param>
         /// <param name="orderer">A comparator method to determine the ordering of
@@ -118,9 +120,9 @@ public static class ObservableCollectionMixin
         public static IReactiveDerivedBindingList<TNew> CreateDerivedBindingList<T, TNew, TDontCare>(
             this IEnumerable<T> This,
             Func<T, TNew> selector,
+            Action<TNew> removed,
             Func<T, bool> filter = null,
             Func<TNew, TNew, int> orderer = null,
-            Action<TNew> removed = null,
             IObservable<TDontCare> signalReset = null)
         {
             Contract.Requires(selector != null);
@@ -134,6 +136,72 @@ public static IReactiveDerivedBindingList<TNew> CreateDerivedBindingList<T, TNew
             return new ReactiveDerivedBindingList<T, TNew>(This, selector, filter, orderer, removed, reset);
         }
 
+        /// <summary>
+        /// Creates a collection whose contents will "follow" another
+        /// collection; this method is useful for creating ViewModel collections
+        /// that are automatically updated when the respective Model collection
+        /// is updated.
+        ///
+        /// Note that even though this method attaches itself to any 
+        /// IEnumerable, it will only detect changes from objects implementing
+        /// INotifyCollectionChanged (like ReactiveList). If your source
+        /// collection doesn't implement this, signalReset is the way to signal
+        /// the derived collection to reorder/refilter itself.
+        /// </summary>
+        /// <param name="selector">A Select function that will be run on each
+        /// item.</param>
+        /// <param name="filter">A filter to determine whether to exclude items 
+        /// in the derived collection.</param>
+        /// <param name="orderer">A comparator method to determine the ordering of
+        /// the resulting collection.</param>
+        /// <param name="signalReset">When this Observable is signalled, 
+        /// the derived collection will be manually 
+        /// reordered/refiltered.</param>
+        /// <returns>A new collection whose items are equivalent to
+        /// Collection.Select().Where().OrderBy() and will mirror changes 
+        /// in the initial collection.</returns>
+        public static IReactiveDerivedBindingList<TNew> CreateDerivedBindingList<T, TNew, TDontCare>(
+            this IEnumerable<T> This,
+            Func<T, TNew> selector,
+            Func<T, bool> filter = null,
+            Func<TNew, TNew, int> orderer = null,
+            IObservable<TDontCare> signalReset = null)
+        {
+            return This.CreateDerivedBindingList(selector, null, filter, orderer, signalReset);
+        }
+
+        /// <summary>
+        /// Creates a collection whose contents will "follow" another
+        /// collection; this method is useful for creating ViewModel collections
+        /// that are automatically updated when the respective Model collection
+        /// is updated.
+        /// 
+        /// Be aware that this overload will result in a collection that *only* 
+        /// updates if the source implements INotifyCollectionChanged. If your
+        /// list changes but isn't a ReactiveList/ObservableCollection,
+        /// you probably want to use the other overload.
+        /// </summary>
+        /// <param name="selector">A Select function that will be run on each
+        /// item.</param>
+        /// /// <param name="onRemoved">An action that is called on each item when
+        /// it is removed.</param>
+        /// <param name="filter">A filter to determine whether to exclude items 
+        /// in the derived collection.</param>
+        /// <param name="orderer">A comparator method to determine the ordering of
+        /// the resulting collection.</param>
+        /// <returns>A new collection whose items are equivalent to
+        /// Collection.Select().Where().OrderBy() and will mirror changes 
+        /// in the initial collection.</returns>
+        public static IReactiveDerivedBindingList<TNew> CreateDerivedBindingList<T, TNew>(
+            this IEnumerable<T> This,
+            Func<T, TNew> selector,
+            Action<TNew> removed,
+            Func<T, bool> filter = null,
+            Func<TNew, TNew, int> orderer = null)
+        {
+            return This.CreateDerivedBindingList(selector, removed, filter, orderer, (IObservable<Unit>)null);
+        }
+
         /// <summary>
         /// Creates a collection whose contents will "follow" another
         /// collection; this method is useful for creating ViewModel collections
@@ -158,10 +226,9 @@ public static IReactiveDerivedBindingList<TNew> CreateDerivedBindingList<T, TNew
             this IEnumerable<T> This,
             Func<T, TNew> selector,
             Func<T, bool> filter = null,
-            Action<TNew> removed = null,
             Func<TNew, TNew, int> orderer = null)
         {
-            return This.CreateDerivedBindingList(selector, filter, orderer, removed, (IObservable<Unit>)null);
+            return This.CreateDerivedBindingList(selector, null, filter, orderer, (IObservable<Unit>)null);
         }
     }
 }

ReactiveUI/ReactiveCollectionMixins.cs

@@ -11,6 +11,7 @@
 using System.Threading;
 using Splat;
 using System.Reactive.Concurrency;
+using System.Linq;
 
 namespace ReactiveUI
 {
@@ -212,25 +213,22 @@ public ReactiveDerivedCollection(
             this.selector = selector;
             this.filter = filter;
             this.orderer = orderer;
-            this.onRemoved = onRemoved;
+            this.onRemoved = onRemoved ?? (_ => { });
             this.signalReset = signalReset;
             this.scheduler = scheduler;
 
             this.inner = new CompositeDisposable();
             this.indexToSourceIndexMap = new List<int>();
             this.sourceCopy = new List<TSource>();
 
-            if (this.onRemoved != null)
-            {
-               this.inner.Add(Disposable.Create(() =>
-               {
-                   foreach (var item in this)
-                   {
-                       this.onRemoved(item);
-                   }
-               }));
-            }
-           
+            if (onRemoved != null) {
+                this.inner.Add(Disposable.Create(() => {
+                    foreach (var item in this) {
+                        this.onRemoved(item);
+                    }
+                }));
+            }
+
             this.addAllItemsFromSourceCollection();
             this.wireUpChangeNotifications();
         }
@@ -382,10 +380,7 @@ void internalReplace(int destinationIndex, TValue newItem)
         {
             var item = this[destinationIndex];
             base.SetItem(destinationIndex, newItem);
-            if (this.onRemoved != null)
-            {
-               onRemoved(item);
-            }
+            onRemoved(item);
         }
 
         /// <summary>
@@ -590,16 +585,14 @@ protected override void internalClear()
         {
             indexToSourceIndexMap.Clear();
             sourceCopy.Clear();
+            
+            var items = this.ToArray();
+            
+            base.internalClear();
 
-            if (this.onRemoved != null)
-            {
-               foreach (var item in this)
-               {
-                  onRemoved(item);
-               }
+            foreach (var item in items) {
+                onRemoved(item);
             }
-
-            base.internalClear();
         }
 
         void internalInsertAndMap(int sourceIndex, TValue value)
@@ -615,10 +608,7 @@ protected override void internalRemoveAt(int destinationIndex)
             indexToSourceIndexMap.RemoveAt(destinationIndex);
             var item = this[destinationIndex];
             base.internalRemoveAt(destinationIndex);
-            if (this.onRemoved != null)
-            {
-               onRemoved(item);
-            }
+            onRemoved(item);
         }
 
         /// <summary>
@@ -889,6 +879,8 @@ public static class ObservableCollectionMixin
         /// </summary>
         /// <param name="selector">A Select function that will be run on each
         /// item.</param>
+        /// <param name="onRemoved">An action that is called on each item when
+        /// it is removed.</param>
         /// <param name="filter">A filter to determine whether to exclude items 
         /// in the derived collection.</param>
         /// <param name="orderer">A comparator method to determine the ordering of
@@ -902,9 +894,9 @@ public static class ObservableCollectionMixin
         public static IReactiveDerivedList<TNew> CreateDerivedCollection<T, TNew, TDontCare>(
             this IEnumerable<T> This,
             Func<T, TNew> selector,
+            Action<TNew> onRemoved,
             Func<T, bool> filter = null,
             Func<TNew, TNew, int> orderer = null,
-            Action<TNew> onRemoved = null,
             IObservable<TDontCare> signalReset = null,
             IScheduler scheduler = null)
         {
@@ -923,6 +915,74 @@ public static IReactiveDerivedList<TNew> CreateDerivedCollection<T, TNew, TDontC
             return new ReactiveDerivedCollection<T, TNew>(This, selector, filter, orderer, onRemoved, reset, scheduler);
         }
 
+        /// <summary>
+        /// Creates a collection whose contents will "follow" another
+        /// collection; this method is useful for creating ViewModel collections
+        /// that are automatically updated when the respective Model collection
+        /// is updated.
+        ///
+        /// Note that even though this method attaches itself to any 
+        /// IEnumerable, it will only detect changes from objects implementing
+        /// INotifyCollectionChanged (like ReactiveList). If your source
+        /// collection doesn't implement this, signalReset is the way to signal
+        /// the derived collection to reorder/refilter itself.
+        /// </summary>
+        /// <param name="selector">A Select function that will be run on each
+        /// item.</param>
+        /// <param name="filter">A filter to determine whether to exclude items 
+        /// in the derived collection.</param>
+        /// <param name="orderer">A comparator method to determine the ordering of
+        /// the resulting collection.</param>
+        /// <param name="signalReset">When this Observable is signalled, 
+        /// the derived collection will be manually 
+        /// reordered/refiltered.</param>
+        /// <returns>A new collection whose items are equivalent to
+        /// Collection.Select().Where().OrderBy() and will mirror changes 
+        /// in the initial collection.</returns>
+        public static IReactiveDerivedList<TNew> CreateDerivedCollection<T, TNew, TDontCare>(
+            this IEnumerable<T> This,
+            Func<T, TNew> selector,
+            Func<T, bool> filter = null,
+            Func<TNew, TNew, int> orderer = null,
+            IObservable<TDontCare> signalReset = null,
+            IScheduler scheduler = null)
+        {
+            return This.CreateDerivedCollection(selector, (Action<TNew>)null, filter, orderer, signalReset, scheduler);
+        }
+
+        /// <summary>
+        /// Creates a collection whose contents will "follow" another
+        /// collection; this method is useful for creating ViewModel collections
+        /// that are automatically updated when the respective Model collection
+        /// is updated.
+        /// 
+        /// Be aware that this overload will result in a collection that *only* 
+        /// updates if the source implements INotifyCollectionChanged. If your
+        /// list changes but isn't a ReactiveList/ObservableCollection,
+        /// you probably want to use the other overload.
+        /// </summary>
+        /// <param name="selector">A Select function that will be run on each
+        /// item.</param>
+        /// <param name="onRemoved">An action that is called on each item when
+        /// it is removed.</param>
+        /// <param name="filter">A filter to determine whether to exclude items 
+        /// in the derived collection.</param>
+        /// <param name="orderer">A comparator method to determine the ordering of
+        /// the resulting collection.</param>
+        /// <returns>A new collection whose items are equivalent to
+        /// Collection.Select().Where().OrderBy() and will mirror changes 
+        /// in the initial collection.</returns>
+        public static IReactiveDerivedList<TNew> CreateDerivedCollection<T, TNew>(
+            this IEnumerable<T> This,
+            Func<T, TNew> selector,
+            Action<TNew> onRemoved,
+            Func<T, bool> filter = null,
+            Func<TNew, TNew, int> orderer = null,
+            IScheduler scheduler = null)
+        {
+            return This.CreateDerivedCollection(selector, onRemoved, filter, orderer, (IObservable<Unit>)null, scheduler);
+        }
+
         /// <summary>
         /// Creates a collection whose contents will "follow" another
         /// collection; this method is useful for creating ViewModel collections
@@ -948,10 +1008,10 @@ public static IReactiveDerivedList<TNew> CreateDerivedCollection<T, TNew>(
             Func<T, TNew> selector,
             Func<T, bool> filter = null,
             Func<TNew, TNew, int> orderer = null,
-            Action<TNew> onRemoved = null,
             IScheduler scheduler = null)
         {
-            return This.CreateDerivedCollection(selector, filter, orderer, onRemoved, (IObservable<Unit>)null, scheduler);
+            return This.CreateDerivedCollection(selector, (Action<TNew>)null, filter, orderer, (IObservable<Unit>)null, scheduler);
         }
+
     }
 }