Ensures CollectionChanged event is subscribed once or none if the source is null. For the ListWrapper the user is responsible to dispose it.

Terminal.Gui/Views/ListView.cs

@@ -6,7 +6,7 @@ using static Terminal.Gui.SpinnerStyle;
 namespace Terminal.Gui;
 
 /// <summary>Implement <see cref="IListDataSource"/> to provide custom rendering for a <see cref="ListView"/>.</summary>
-public interface IListDataSource
+public interface IListDataSource: IDisposable
 {
     /// <summary>
     /// Event to raise when an item is added, removed, or moved, or the entire list is refreshed.
@@ -265,10 +265,11 @@ public class ListView : View
         set
         {
             if (_source == value)
-
             {
                 return;
             }
+
+            _source?.Dispose ();
             _source = value;
 
             if (_source is { })
@@ -884,13 +885,21 @@ public class ListView : View
     /// </summary>
     /// <param name="e"></param>
     protected virtual void OnCollectionChanged (NotifyCollectionChangedEventArgs e) { CollectionChanged?.Invoke (this, e); }
+
+    /// <inheritdoc />
+    protected override void Dispose (bool disposing)
+    {
+        _source?.Dispose ();
+
+        base.Dispose (disposing);
+    }
 }
 
 /// <summary>
 ///     Provides a default implementation of <see cref="IListDataSource"/> that renders <see cref="ListView"/> items
 ///     using <see cref="object.ToString()"/>.
 /// </summary>
-public class ListWrapper<T> : IListDataSource
+public class ListWrapper<T> : IListDataSource, IDisposable
 {
     private int _count;
     private BitArray _marks;
@@ -904,15 +913,17 @@ public class ListWrapper<T> : IListDataSource
             _count = source.Count;
             _marks = new BitArray (_count);
             _source = source;
-            _source.CollectionChanged += (s, e) =>
-                                         {
-                                             CheckAndResizeMarksIfRequired ();
-                                             CollectionChanged?.Invoke (s, e);
-                                         };
+            _source.CollectionChanged += Source_CollectionChanged;
             Length = GetMaxLengthItem ();
         }
     }
 
+    private void Source_CollectionChanged (object sender, NotifyCollectionChangedEventArgs e)
+    {
+        CheckAndResizeMarksIfRequired ();
+        CollectionChanged?.Invoke (sender, e);
+    }
+
     /// <inheritdoc />
     public event NotifyCollectionChangedEventHandler CollectionChanged;
 
@@ -1076,4 +1087,13 @@ public class ListWrapper<T> : IListDataSource
             driver.AddRune ((Rune)' ');
         }
     }
+
+    /// <inheritdoc />
+    public void Dispose ()
+    {
+        if (_source is { })
+        {
+            _source.CollectionChanged -= Source_CollectionChanged;
+        }
+    }
 }

UICatalog/Scenarios/ListViewWithSelection.cs

@@ -160,21 +160,21 @@ public class ListViewWithSelection : Scenario
     internal class ScenarioListDataSource : IListDataSource
     {
         private readonly int _nameColumnWidth = 30;
-        private int count;
-        private BitArray marks;
-        private ObservableCollection<Scenario> scenarios;
+        private int _count;
+        private BitArray _marks;
+        private ObservableCollection<Scenario> _scenarios;
         public ScenarioListDataSource (ObservableCollection<Scenario> itemList) { Scenarios = itemList; }
 
         public ObservableCollection<Scenario> Scenarios
         {
-            get => scenarios;
+            get => _scenarios;
             set
             {
                 if (value != null)
                 {
-                    count = value.Count;
-                    marks = new BitArray (count);
-                    scenarios = value;
+                    _count = value.Count;
+                    _marks = new BitArray (_count);
+                    _scenarios = value;
                     Length = GetMaxLengthItem ();
                 }
             }
@@ -182,9 +182,9 @@ public class ListViewWithSelection : Scenario
 
         public bool IsMarked (int item)
         {
-            if (item >= 0 && item < count)
+            if (item >= 0 && item < _count)
             {
-                return marks [item];
+                return _marks [item];
             }
 
             return false;
@@ -218,9 +218,9 @@ public class ListViewWithSelection : Scenario
 
         public void SetMark (int item, bool value)
         {
-            if (item >= 0 && item < count)
+            if (item >= 0 && item < _count)
             {
-                marks [item] = value;
+                _marks [item] = value;
             }
         }
 
@@ -228,17 +228,17 @@ public class ListViewWithSelection : Scenario
 
         private int GetMaxLengthItem ()
         {
-            if (scenarios?.Count == 0)
+            if (_scenarios?.Count == 0)
             {
                 return 0;
             }
 
             var maxLength = 0;
 
-            for (var i = 0; i < scenarios.Count; i++)
+            for (var i = 0; i < _scenarios.Count; i++)
             {
                 string s = string.Format (
-                                          string.Format ("{{0,{0}}}", -_nameColumnWidth),
+                                          $"{{0,{-_nameColumnWidth}}}",
                                           Scenarios [i].GetName ()
                                          );
                 var sc = $"{s}  {Scenarios [i].GetDescription ()}";
@@ -280,5 +280,10 @@ public class ListViewWithSelection : Scenario
                 used++;
             }
         }
+
+        public void Dispose ()
+        {
+            _scenarios = null;
+        }
     }
 }

UnitTests/Views/ListViewTests.cs

@@ -674,6 +674,7 @@ Item 6",
     {
         /// <inheritdoc />
         public event NotifyCollectionChangedEventHandler CollectionChanged;
+
         public int Count => 0;
         public int Length => 0;
         public bool IsMarked (int item) { throw new NotImplementedException (); }
@@ -694,6 +695,11 @@ Item 6",
 
         public void SetMark (int item, bool value) { throw new NotImplementedException (); }
         public IList ToList () { return new List<string> { "One", "Two", "Three" }; }
+
+        public void Dispose ()
+        {
+            throw new NotImplementedException ();
+        }
     }
 
     [Fact]
@@ -824,9 +830,228 @@ Item 6",
         {
             source.Remove (source [0]);
         }
-
         Assert.Equal (0, added);
         Assert.Equal (3, removed);
         Assert.Empty (source);
     }
-}
+
+    [Fact]
+    public void CollectionChanged_Event_Is_Only_Subscribed_Once ()
+    {
+        var added = 0;
+        var removed = 0;
+        var otherActions = 0;
+        ObservableCollection<string> source1 = [];
+        var lv = new ListView { Source = new ListWrapper<string> (source1) };
+
+        lv.CollectionChanged += (sender, args) =>
+                                {
+                                    if (args.Action == NotifyCollectionChangedAction.Add)
+                                    {
+                                        added++;
+                                    }
+                                    else if (args.Action == NotifyCollectionChangedAction.Remove)
+                                    {
+                                        removed++;
+                                    }
+                                    else
+                                    {
+                                        otherActions++;
+                                    }
+                                };
+
+        ObservableCollection<string> source2 = [];
+        lv.Source = new ListWrapper<string> (source2);
+        ObservableCollection<string> source3 = [];
+        lv.Source = new ListWrapper<string> (source3);
+        Assert.Equal (0, added);
+        Assert.Equal (0, removed);
+        Assert.Equal (0, otherActions);
+
+        for (int i = 0; i < 3; i++)
+        {
+            source1.Add ($"Item{i}");
+            source2.Add ($"Item{i}");
+            source3.Add ($"Item{i}");
+        }
+        Assert.Equal (3, added);
+        Assert.Equal (0, removed);
+        Assert.Equal (0, otherActions);
+
+        added = 0;
+
+        for (int i = 0; i < 3; i++)
+        {
+            source1.Remove (source1 [0]);
+            source2.Remove (source2 [0]);
+            source3.Remove (source3 [0]);
+        }
+        Assert.Equal (0, added);
+        Assert.Equal (3, removed);
+        Assert.Equal (0, otherActions);
+        Assert.Empty (source1);
+        Assert.Empty (source2);
+        Assert.Empty (source3);
+    }
+
+    [Fact]
+    public void CollectionChanged_Event_UnSubscribe_Previous_If_New_Is_Null ()
+    {
+        var added = 0;
+        var removed = 0;
+        var otherActions = 0;
+        ObservableCollection<string> source1 = [];
+        var lv = new ListView { Source = new ListWrapper<string> (source1) };
+
+        lv.CollectionChanged += (sender, args) =>
+        {
+            if (args.Action == NotifyCollectionChangedAction.Add)
+            {
+                added++;
+            }
+            else if (args.Action == NotifyCollectionChangedAction.Remove)
+            {
+                removed++;
+            }
+            else
+            {
+                otherActions++;
+            }
+        };
+
+        lv.Source = new ListWrapper<string> (null);
+        Assert.Equal (0, added);
+        Assert.Equal (0, removed);
+        Assert.Equal (0, otherActions);
+
+        for (int i = 0; i < 3; i++)
+        {
+            source1.Add ($"Item{i}");
+        }
+        Assert.Equal (0, added);
+        Assert.Equal (0, removed);
+        Assert.Equal (0, otherActions);
+
+        for (int i = 0; i < 3; i++)
+        {
+            source1.Remove (source1 [0]);
+        }
+        Assert.Equal (0, added);
+        Assert.Equal (0, removed);
+        Assert.Equal (0, otherActions);
+        Assert.Empty (source1);
+    }
+
+    [Fact]
+    public void ListWrapper_CollectionChanged_Event_Is_Only_Subscribed_Once ()
+    {
+        var added = 0;
+        var removed = 0;
+        var otherActions = 0;
+        ObservableCollection<string> source1 = [];
+        ListWrapper<string> lw = new (source1);
+
+        lw.CollectionChanged += (sender, args) =>
+        {
+            if (args.Action == NotifyCollectionChangedAction.Add)
+            {
+                added++;
+            }
+            else if (args.Action == NotifyCollectionChangedAction.Remove)
+            {
+                removed++;
+            }
+            else
+            {
+                otherActions++;
+            }
+        };
+
+        ObservableCollection<string> source2 = [];
+        lw = new (source2);
+        ObservableCollection<string> source3 = [];
+        lw = new (source3);
+        Assert.Equal (0, added);
+        Assert.Equal (0, removed);
+        Assert.Equal (0, otherActions);
+
+        for (int i = 0; i < 3; i++)
+        {
+            source1.Add ($"Item{i}");
+            source2.Add ($"Item{i}");
+            source3.Add ($"Item{i}");
+        }
+
+        Assert.Equal (3, added);
+        Assert.Equal (0, removed);
+        Assert.Equal (0, otherActions);
+
+        added = 0;
+
+        for (int i = 0; i < 3; i++)
+        {
+            source1.Remove (source1 [0]);
+            source2.Remove (source2 [0]);
+            source3.Remove (source3 [0]);
+        }
+        Assert.Equal (0, added);
+        Assert.Equal (3, removed);
+        Assert.Equal (0, otherActions);
+        Assert.Empty (source1);
+        Assert.Empty (source2);
+        Assert.Empty (source3);
+    }
+
+    [Fact]
+    public void ListWrapper_CollectionChanged_Event_UnSubscribe_Previous_If_New_Is_Null ()
+    {
+        var added = 0;
+        var removed = 0;
+        var otherActions = 0;
+        ObservableCollection<string> source1 = [];
+        ListWrapper<string> lw = new (source1);
+
+        lw.CollectionChanged += Lw_CollectionChanged;
+
+        lw.Dispose ();
+        lw = new (null);
+        Assert.Equal (0, lw.Count);
+        Assert.Equal (0, added);
+        Assert.Equal (0, removed);
+        Assert.Equal (0, otherActions);
+
+        for (int i = 0; i < 3; i++)
+        {
+            source1.Add ($"Item{i}");
+        }
+        Assert.Equal (0, added);
+        Assert.Equal (0, removed);
+        Assert.Equal (0, otherActions);
+
+        for (int i = 0; i < 3; i++)
+        {
+            source1.Remove (source1 [0]);
+        }
+        Assert.Equal (0, added);
+        Assert.Equal (0, removed);
+        Assert.Equal (0, otherActions);
+        Assert.Empty (source1);
+
+
+        void Lw_CollectionChanged (object sender, NotifyCollectionChangedEventArgs e)
+        {
+            if (e.Action == NotifyCollectionChangedAction.Add)
+            {
+                added++;
+            }
+            else if (e.Action == NotifyCollectionChangedAction.Remove)
+            {
+                removed++;
+            }
+            else
+            {
+                otherActions++;
+            }
+        }
+    }
+}