Unify Array/List FindIndex/FindLastIndex handling and argument checking. Fixes #14632
authorMarek Safar <marek.safar@gmail.com>
Fri, 13 Sep 2013 12:49:23 +0000 (14:49 +0200)
committerMarek Safar <marek.safar@gmail.com>
Fri, 13 Sep 2013 12:51:08 +0000 (14:51 +0200)
mcs/class/corlib/System.Collections.Generic/List.cs
mcs/class/corlib/System/Array.cs
mcs/class/corlib/Test/System.Collections.Generic/ListTest.cs
mcs/class/corlib/Test/System/ArrayTest.cs

index 17056a1b21c37a4c60dbfc63f7548b7b258112d4..509c4033ce2af46513ff561286f6d707efd2c585 100644 (file)
@@ -116,6 +116,18 @@ namespace System.Collections.Generic {
                                throw new ArgumentException ("index and count exceed length of list");
                }
                
+               void CheckRangeOutOfRange (int idx, int count)
+               {
+                       if (idx < 0)
+                               throw new ArgumentOutOfRangeException ("index");
+                       
+                       if (count < 0)
+                               throw new ArgumentOutOfRangeException ("count");
+
+                       if ((uint) idx + (uint) count > (uint) _size)
+                               throw new ArgumentOutOfRangeException ("index and count exceed length of list");
+               }
+
                void AddCollection (ICollection <T> collection)
                {
                        int collectionCount = collection.Count;
@@ -212,14 +224,25 @@ namespace System.Collections.Generic {
                public bool Exists (Predicate <T> match)
                {
                        CheckMatch(match);
-                       return GetIndex(0, _size, match) != -1;
+
+                       foreach (var t in _items)
+                               if (match (t))
+                                       return true;
+
+                       return false;
                }
                
                public T Find (Predicate <T> match)
                {
                        CheckMatch(match);
-                       int i = GetIndex(0, _size, match);
-                       return (i != -1) ? _items [i] : default (T);
+
+                       for (int i = 0; i < _size; i++) {
+                               var item = _items [i];
+                               if (match (item))
+                                       return item;
+                       }
+
+                       return default (T);
                }
                
                static void CheckMatch (Predicate <T> match)
@@ -296,68 +319,56 @@ namespace System.Collections.Generic {
                public int FindIndex (Predicate <T> match)
                {
                        CheckMatch (match);
-                       return GetIndex (0, _size, match);
+                       return Array.GetIndex (_items, 0, _size, match);
                }
                
                public int FindIndex (int startIndex, Predicate <T> match)
                {
                        CheckMatch (match);
-                       CheckIndex (startIndex);
-                       return GetIndex (startIndex, _size - startIndex, match);
+                       CheckStartIndex (startIndex);
+                       return Array.GetIndex (_items, startIndex, _size - startIndex, match);
                }
                public int FindIndex (int startIndex, int count, Predicate <T> match)
                {
                        CheckMatch (match);
-                       CheckRange (startIndex, count);
-                       return GetIndex (startIndex, count, match);
-               }
-               int GetIndex (int startIndex, int count, Predicate <T> match)
-               {
-                       int end = startIndex + count;
-                       for (int i = startIndex; i < end; i ++)
-                               if (match (_items [i]))
-                                       return i;
-                               
-                       return -1;
+                       CheckRangeOutOfRange (startIndex, count);
+                       return Array.GetIndex (_items, startIndex, count, match);
                }
                
                public T FindLast (Predicate <T> match)
                {
                        CheckMatch (match);
-                       int i = GetLastIndex (0, _size, match);
+                       int i = Array.GetLastIndex (_items, 0, _size, match);
                        return i == -1 ? default (T) : this [i];
                }
                
                public int FindLastIndex (Predicate <T> match)
                {
                        CheckMatch (match);
-                       return GetLastIndex (0, _size, match);
+                       return Array.GetLastIndex (_items, 0, _size, match);
                }
                
                public int FindLastIndex (int startIndex, Predicate <T> match)
                {
                        CheckMatch (match);
-                       CheckIndex (startIndex);
-                       return GetLastIndex (0, startIndex + 1, match);
+                       CheckStartIndex (startIndex);
+                       return Array.GetLastIndex (_items, 0, startIndex + 1, match);
                }
                
                public int FindLastIndex (int startIndex, int count, Predicate <T> match)
                {
                        CheckMatch (match);
-                       int start = startIndex - count + 1;
-                       CheckRange (start, count);
-                       return GetLastIndex (start, count, match);
-               }
+                       CheckStartIndex (startIndex);
+                       
+                       if (count < 0)
+                               throw new ArgumentOutOfRangeException ("count");
 
-               int GetLastIndex (int startIndex, int count, Predicate <T> match)
-               {
-                       // unlike FindLastIndex, takes regular params for search range
-                       for (int i = startIndex + count; i != startIndex;)
-                               if (match (_items [--i]))
-                                       return i;
-                       return -1;      
+                       if (startIndex - count + 1 < 0)
+                               throw new ArgumentOutOfRangeException ("count must refer to a location within the collection");
+
+                       return Array.GetLastIndex (_items, startIndex - count + 1, count, match);
                }
-               
+
                public void ForEach (Action <T> action)
                {
                        if (action == null)
@@ -423,6 +434,12 @@ namespace System.Collections.Generic {
                        if (index < 0 || (uint) index > (uint) _size)
                                throw new ArgumentOutOfRangeException ("index");
                }
+
+               void CheckStartIndex (int index)
+               {
+                       if (index < 0 || (uint) index > (uint) _size)
+                               throw new ArgumentOutOfRangeException ("startIndex");
+               }
                
                public void Insert (int index, T item)
                {
index 91654019f146e6f8dde8272b6ca8848ad29a540c..756378c56d850d4f7e8fbf79821b0889d72656cd 100644 (file)
@@ -2864,32 +2864,54 @@ namespace System
                {
                        if (array == null)
                                throw new ArgumentNullException ("array");
+
+                       if (match == null)
+                               throw new ArgumentNullException ("match");
                        
-                       return FindLastIndex<T> (array, 0, array.Length, match);
+                       return GetLastIndex (array, 0, array.Length, match);
                }
                
                public static int FindLastIndex<T> (T [] array, int startIndex, Predicate<T> match)
                {
                        if (array == null)
                                throw new ArgumentNullException ();
+
+                       if (startIndex < 0 || (uint) startIndex > (uint) array.Length)
+                               throw new ArgumentOutOfRangeException ("startIndex");
+
+                       if (match == null)
+                               throw new ArgumentNullException ("match");
                        
-                       return FindLastIndex<T> (array, startIndex, array.Length - startIndex, match);
+                       return GetLastIndex (array, 0, startIndex + 1, match);
                }
                
                public static int FindLastIndex<T> (T [] array, int startIndex, int count, Predicate<T> match)
                {
                        if (array == null)
                                throw new ArgumentNullException ("array");
+
                        if (match == null)
                                throw new ArgumentNullException ("match");
+
+                       if (startIndex < 0 || (uint) startIndex > (uint) array.Length)
+                               throw new ArgumentOutOfRangeException ("startIndex");
                        
-                       if (startIndex > array.Length || startIndex + count > array.Length)
-                               throw new ArgumentOutOfRangeException ();
-                       
-                       for (int i = startIndex + count - 1; i >= startIndex; i--)
-                               if (match (array [i]))
+                       if (count < 0)
+                               throw new ArgumentOutOfRangeException ("count");
+
+                       if (startIndex - count + 1 < 0)
+                               throw new ArgumentOutOfRangeException ("count must refer to a location within the array");
+
+                       return GetLastIndex (array, startIndex - count + 1, count, match);
+               }
+
+               internal static int GetLastIndex<T> (T[] array, int startIndex, int count, Predicate<T> match)
+               {
+                       // unlike FindLastIndex, takes regular params for search range
+                       for (int i = startIndex + count; i != startIndex;)
+                               if (match (array [--i]))
                                        return i;
-                               
+
                        return -1;
                }
                
@@ -2897,16 +2919,25 @@ namespace System
                {
                        if (array == null)
                                throw new ArgumentNullException ("array");
+
+                       if (match == null)
+                               throw new ArgumentNullException ("match");
                        
-                       return FindIndex<T> (array, 0, array.Length, match);
+                       return GetIndex (array, 0, array.Length, match);
                }
                
                public static int FindIndex<T> (T [] array, int startIndex, Predicate<T> match)
                {
                        if (array == null)
                                throw new ArgumentNullException ("array");
-                       
-                       return FindIndex<T> (array, startIndex, array.Length - startIndex, match);
+
+                       if (startIndex < 0 || (uint) startIndex > (uint) array.Length)
+                               throw new ArgumentOutOfRangeException ("startIndex");
+
+                       if (match == null)
+                               throw new ArgumentNullException ("match");
+
+                       return GetIndex (array, startIndex, array.Length - startIndex, match);
                }
                
                public static int FindIndex<T> (T [] array, int startIndex, int count, Predicate<T> match)
@@ -2914,13 +2945,22 @@ namespace System
                        if (array == null)
                                throw new ArgumentNullException ("array");
                        
-                       if (match == null)
-                               throw new ArgumentNullException ("match");
+                       if (startIndex < 0)
+                               throw new ArgumentOutOfRangeException ("startIndex");
                        
-                       if (startIndex > array.Length || startIndex + count > array.Length)
-                               throw new ArgumentOutOfRangeException ();
-                       
-                       for (int i = startIndex; i < startIndex + count; i ++)
+                       if (count < 0)
+                               throw new ArgumentOutOfRangeException ("count");
+
+                       if ((uint) startIndex + (uint) count > (uint) array.Length)
+                               throw new ArgumentOutOfRangeException ("index and count exceed length of list");
+
+                       return GetIndex (array, startIndex, count, match);
+               }
+
+               internal static int GetIndex<T> (T[] array, int startIndex, int count, Predicate<T> match)
+               {
+                       int end = startIndex + count;
+                       for (int i = startIndex; i < end; i ++)
                                if (match (array [i]))
                                        return i;
                                
index cb5b9f7cf1c42253f1699ffade7d296a23660f1a..6542238daedae949f3c822ac4f846c3b6a851f42 100644 (file)
@@ -555,12 +555,55 @@ namespace MonoTests.System.Collections.Generic {
 
                        i = _list1.FindIndex (FindMultipleOfTwelve);
                        Assert.AreEqual (-1, i);
+
+                       var a = new List<int> () { 2, 2, 2, 3, 2 };
+                       Assert.AreEqual (2, a.FindIndex (2, 2, l => true));
                }
 
-               [Test, ExpectedException (typeof (ArgumentNullException))]
-               public void FindIndexNullTest ()
+               [Test]
+               public void FindIndex_Invalid ()
                {
-                       int i = _list1.FindIndex (null);
+                       try {
+                               _list1.FindIndex (null);
+                               Assert.Fail ("#1");
+                       } catch (ArgumentNullException) {
+                       }
+
+                       try {
+                               _list1.FindIndex (-1, l => true);
+                               Assert.Fail ("#2");
+                       } catch (ArgumentOutOfRangeException) {
+                       }
+
+                       try {
+                               _list1.FindIndex (-1, 0, l => true);
+                               Assert.Fail ("#2b");
+                       } catch (ArgumentOutOfRangeException) {
+                       }
+
+                       try {
+                               _list1.FindIndex (0, -1, l => true);
+                               Assert.Fail ("#3");
+                       } catch (ArgumentOutOfRangeException) {
+                       }
+
+                       try {
+                               _list1.FindIndex (100, l => true);
+                               Assert.Fail ("#4");
+                       } catch (ArgumentOutOfRangeException) {
+                       }
+
+                       try {
+                               _list1.FindIndex (100, 0, l => true);
+                               Assert.Fail ("#4b");
+                       } catch (ArgumentOutOfRangeException) {
+                       }
+
+                       try {
+                               _list1.FindIndex (7, 2, l => true);
+                               Assert.Fail ("#5");
+                       } catch (ArgumentOutOfRangeException) {
+                       }
                }
 
                [Test]
@@ -579,8 +622,6 @@ namespace MonoTests.System.Collections.Generic {
                        int i = _list1.FindLast (null);
                }
 
-               // FIXME currently generates Invalid IL Code error
-               /*
                [Test]
                public void ForEachTest ()
                {
@@ -589,7 +630,7 @@ namespace MonoTests.System.Collections.Generic {
 
                        Assert.AreEqual (418, i);
                }
-               */
+
                [Test]
                public void FindLastIndexTest ()
                {
@@ -601,12 +642,56 @@ namespace MonoTests.System.Collections.Generic {
 
                        i = _list1.FindIndex (FindMultipleOfTwelve);
                        Assert.AreEqual (-1, i);
+
+                       Assert.AreEqual (2, _list1.FindLastIndex (2, 3, l => true));
+                       Assert.AreEqual (2, _list1.FindLastIndex (2, 2, l => true));
+                       Assert.AreEqual (1, _list1.FindLastIndex (1, 2, l => true));
                }
 
-               [Test, ExpectedException (typeof (ArgumentNullException))]
-               public void FindLastIndexNullTest ()
+               [Test]
+               public void FindLastIndex_Invalid ()
                {
-                       int i = _list1.FindLastIndex (null);
+                       try {
+                               _list1.FindLastIndex (null);
+                               Assert.Fail ("#1");
+                       } catch (ArgumentNullException) {
+                       }
+
+                       try {
+                               _list1.FindLastIndex (-1, l => true);
+                               Assert.Fail ("#2");
+                       } catch (ArgumentOutOfRangeException) {
+                       }
+
+                       try {
+                               _list1.FindLastIndex (-1, 0, l => true);
+                               Assert.Fail ("#2b");
+                       } catch (ArgumentOutOfRangeException) {
+                       }
+
+                       try {
+                               _list1.FindLastIndex (0, -1, l => true);
+                               Assert.Fail ("#3");
+                       } catch (ArgumentOutOfRangeException) {
+                       }
+
+                       try {
+                               _list1.FindLastIndex (100, l => true);
+                               Assert.Fail ("#4");
+                       } catch (ArgumentOutOfRangeException) {
+                       }
+
+                       try {
+                               _list1.FindLastIndex (100, 0, l => true);
+                               Assert.Fail ("#4b");
+                       } catch (ArgumentOutOfRangeException) {
+                       }
+
+                       try {
+                               _list1.FindLastIndex (2, 4, l => true);
+                               Assert.Fail ("#5");
+                       } catch (ArgumentOutOfRangeException) {
+                       }
                }
 
                [Test]
index 4c74ee9d3d447f36f1acebe0d60d2c0ec11310a5..ef41ce17946377d9b7b323e043df55c0a8922573 100644 (file)
@@ -1618,6 +1618,126 @@ public class ArrayTest
                }
        }
 
+
+       [Test]
+       public void FindIndexTest ()
+       {
+               var a = new int[] { 2, 2, 2, 3, 2 };
+               Assert.AreEqual (2, Array.FindIndex (a, 2, 2, l => true));
+       }
+
+       [Test]
+       public void FindIndex_Invalid ()
+       {
+               var array = new int [] { 1, 2, 3, 4, 5 };
+
+               try {
+                       Array.FindIndex (array, null);
+                       Assert.Fail ("#1");
+               } catch (ArgumentNullException) {
+               }
+
+               try {
+                       Array.FindIndex (array, -1, l => true);
+                       Assert.Fail ("#2");
+               } catch (ArgumentOutOfRangeException) {
+               }
+
+               try {
+                       Array.FindIndex (array, -1, 0, l => true);
+                       Assert.Fail ("#2b");
+               } catch (ArgumentOutOfRangeException) {
+               }
+
+               try {
+                       Array.FindIndex (array, 0, -1, l => true);
+                       Assert.Fail ("#3");
+               } catch (ArgumentOutOfRangeException) {
+               }
+
+               try {
+                       Array.FindIndex (array, 100, l => true);
+                       Assert.Fail ("#4");
+               } catch (ArgumentOutOfRangeException) {
+               }
+
+               try {
+                       Array.FindIndex (array, 100, 0, l => true);
+                       Assert.Fail ("#4b");
+               } catch (ArgumentOutOfRangeException) {
+               }
+
+               try {
+                       Array.FindIndex (array, 7, 2, l => true);
+                       Assert.Fail ("#5");
+               } catch (ArgumentOutOfRangeException) {
+               }
+       }
+
+       [Test, ExpectedException (typeof (ArgumentNullException))]
+       public void FindLastNullTest ()
+       {
+               var array = new int [] { 1, 2, 3, 4, 5 };               
+               Array.FindLast (array, null);
+       }
+
+       [Test]
+       public void FindLastIndexTest ()
+       {
+               var array = new int [] { 1, 2, 3, 4, 5 };
+
+               Assert.AreEqual (2, Array.FindLastIndex (array, 2, 3, l => true));
+               Assert.AreEqual (2, Array.FindLastIndex (array, 2, 2, l => true));
+               Assert.AreEqual (1, Array.FindLastIndex (array, 1, 2, l => true));
+       }
+
+       [Test]
+       public void FindLastIndex_Invalid ()
+       {
+               var array = new int [] { 1, 2, 3, 4, 5 };
+               try {
+                       Array.FindLastIndex (array, null);
+                       Assert.Fail ("#1");
+               } catch (ArgumentNullException) {
+               }
+
+               try {
+                       Array.FindLastIndex (array, -1, l => true);
+                       Assert.Fail ("#2");
+               } catch (ArgumentOutOfRangeException) {
+               }
+
+               try {
+                       Array.FindLastIndex (array, -1, 0, l => true);
+                       Assert.Fail ("#2b");
+               } catch (ArgumentOutOfRangeException) {
+               }
+
+               try {
+                       Array.FindLastIndex (array, 0, -1, l => true);
+                       Assert.Fail ("#3");
+               } catch (ArgumentOutOfRangeException) {
+               }
+
+               try {
+                       Array.FindLastIndex (array, 100, l => true);
+                       Assert.Fail ("#4");
+               } catch (ArgumentOutOfRangeException) {
+               }
+
+               try {
+                       Array.FindLastIndex (array, 100, 0, l => true);
+                       Assert.Fail ("#4b");
+               } catch (ArgumentOutOfRangeException) {
+               }
+
+               try {
+                       Array.FindLastIndex (array, 2, 4, l => true);
+                       Assert.Fail ("#5");
+               } catch (ArgumentOutOfRangeException) {
+               }
+       }
+
        [Test]
        public void TestReverse() {
                {