IList to IEnumerable changes - attempts to fix #38 - includes a small…

… performance warning!
MvvmCross · Oct 15, 2012 · 2b2be98 · 2b2be98
1 parent 6ad6c36
commit 2b2be98
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 15 deletions.
diff --git a/Cirrious/Cirrious.MvvmCross.Binding.Droid/Views/MvxBindableAutoCompleteTextView.cs b/Cirrious/Cirrious.MvvmCross.Binding.Droid/Views/MvxBindableAutoCompleteTextView.cs
@@ -40,7 +40,7 @@ public MvxBindableAutoCompleteTextView(Context context, IAttributeSet attrs, Mvx
         private void OnItemClick(object sender, AdapterView.ItemClickEventArgs itemClickEventArgs)
         {
             MvxTrace.Trace("Item clicked {0}", itemClickEventArgs.Position);
-            var selectedObject = Adapter.ItemsSource[itemClickEventArgs.Position];
+            var selectedObject = Adapter.GetSourceItem(itemClickEventArgs.Position);
             MvxTrace.Trace("Item is {0}", selectedObject);
             SelectedObject = selectedObject;
         }
@@ -76,7 +76,7 @@ private void AdapterOnPartialTextChanged(object sender, EventArgs eventArgs)
         }
 
         [MvxSetToNullAfterBinding]
-        public IList ItemsSource
+        public IEnumerable ItemsSource
         {
             get { return Adapter.ItemsSource; }
             set { Adapter.ItemsSource = value; }

diff --git a/Cirrious/Cirrious.MvvmCross.Binding.Droid/Views/MvxBindableLinearLayout.cs b/Cirrious/Cirrious.MvvmCross.Binding.Droid/Views/MvxBindableLinearLayout.cs
@@ -44,7 +44,7 @@ private void OnChildViewRemoved(object sender, ChildViewRemovedEventArgs childVi
         public MvxBindableListAdapterWithChangedEvent Adapter { get; set; }
 
         [MvxSetToNullAfterBinding]
-        public IList ItemsSource
+        public IEnumerable ItemsSource
         {
             get { return Adapter.ItemsSource; }
             set { Adapter.ItemsSource = value; }

diff --git a/Cirrious/Cirrious.MvvmCross.Binding.Droid/Views/MvxBindableListAdapter.cs b/Cirrious/Cirrious.MvvmCross.Binding.Droid/Views/MvxBindableListAdapter.cs
@@ -30,7 +30,7 @@ public class MvxBindableListAdapter
         private readonly Context _context;
         private int _itemTemplateId;
         private int _dropDownItemTemplateId;
-        private IList _itemsSource;
+        private IEnumerable _itemsSource;
 
         public MvxBindableListAdapter(Context context)
         {
@@ -47,7 +47,7 @@ public MvxBindableListAdapter(Context context)
         public int SimpleViewLayoutId { get; set; }
 
         [MvxSetToNullAfterBinding]
-        public IList ItemsSource
+        public IEnumerable ItemsSource
         {
             get { return _itemsSource; }
             set {
@@ -92,18 +92,33 @@ public override int Count
                 if (_itemsSource == null)
                     return 0;
 
-                return _itemsSource.Count;
+                var itemsList = _itemsSource as IList;
+                if (itemsList != null)
+                {
+                    return itemsList.Count;
+                }
+
+                var enumerator = _itemsSource.GetEnumerator();
+                var count = 0;
+                while (enumerator.MoveNext())
+                {
+                    count++;
+                }
+
+                return count;
             }
         }
 
-        protected virtual void SetItemsSource(IList value)
+        protected virtual void SetItemsSource(IEnumerable value)
         {
             if (_itemsSource == value)
                 return;
             var existingObservable = _itemsSource as INotifyCollectionChanged;
             if (existingObservable != null)
                 existingObservable.CollectionChanged -= OnItemsSourceCollectionChanged;
             _itemsSource = value;
+            if (_itemsSource != null && _itemsSource is IList)
+                MvxBindingTrace.Trace(MvxTraceLevel.Warning, "Binding to IEnumerable rather than IList - this can be inefficient, especially for large lists");
             var newObservable = _itemsSource as INotifyCollectionChanged;
             if (newObservable != null)
                 newObservable.CollectionChanged += OnItemsSourceCollectionChanged;
@@ -124,14 +139,47 @@ public int GetPosition(object item)
         {
             if (_itemsSource == null)
                 return -1;
-            return _itemsSource.IndexOf(item);
+
+            var itemsList = _itemsSource as IList;
+            if (itemsList != null)
+            {
+                return itemsList.IndexOf(item);
+            }
+
+            var enumerator = _itemsSource.GetEnumerator();
+            for (var i = 0; ; i++)
+            {
+                if (!enumerator.MoveNext())
+                    return -1;
+
+                if (enumerator.Current == item)
+                    return i;
+            }
         }
 
-        public override Object GetItem(int position)
+        public System.Object GetSourceItem(int position)
         {
             if (_itemsSource == null)
                 return null;
-            return new MvxJavaContainer<object>(_itemsSource[position]);
+
+            var itemsList = _itemsSource as IList;
+            if (itemsList != null)
+            {
+                return itemsList[position];
+            }
+
+            var enumerator = _itemsSource.GetEnumerator();
+            for (var i = 0; i <= position; i++)
+            {
+                enumerator.MoveNext();
+            }
+
+            return enumerator.Current;
+        }
+
+        public override Object GetItem(int position)
+        {
+            return new MvxJavaContainer<object>(GetSourceItem(position));
         }
 
         public override long GetItemId(int position)
@@ -157,13 +205,16 @@ private View GetView(int position, View convertView, ViewGroup parent, int templ
                 return null;
             }
 
-            if (position < 0 || position >= _itemsSource.Count)
+            // 15 Oct 2012 - this position check removed as it's inefficient, especially for IEnumerable based collections
+            /*
+            if (position < 0 || position >= Count)
             {
-                MvxBindingTrace.Trace(MvxTraceLevel.Error, "GetView called with invalid Position - zero indexed {0} out of {1}", position, _itemsSource.Count);
+                MvxBindingTrace.Trace(MvxTraceLevel.Error, "GetView called with invalid Position - zero indexed {0} out of {1}", position, Count);
                 return null;
             }
+            */
 
-            var source = _itemsSource[position];
+            var source = GetSourceItem(position);
 
             return GetBindableView(convertView, source, templateId);
         }

diff --git a/Cirrious/Cirrious.MvvmCross.Binding.Droid/Views/MvxBindableListView.cs b/Cirrious/Cirrious.MvvmCross.Binding.Droid/Views/MvxBindableListView.cs
@@ -56,7 +56,7 @@ public MvxBindableListView(Context context, IAttributeSet attrs, MvxBindableList
         }
 
         [MvxSetToNullAfterBinding]
-        public IList ItemsSource
+        public IEnumerable ItemsSource
         {
             get { return Adapter.ItemsSource; }
             set { Adapter.ItemsSource = value; }

diff --git a/Cirrious/Cirrious.MvvmCross.Binding.Droid/Views/MvxBindableSpinner.cs b/Cirrious/Cirrious.MvvmCross.Binding.Droid/Views/MvxBindableSpinner.cs
@@ -45,7 +45,7 @@ public MvxBindableSpinner(Context context, IAttributeSet attrs, MvxBindableListA
         }
 
         [MvxSetToNullAfterBinding]
-        public IList ItemsSource
+        public IEnumerable ItemsSource
         {
             get { return Adapter.ItemsSource; }
             set { Adapter.ItemsSource = value; }