Collection<T>: Validate parameters in the public methods (#23166)
authorJustin Van Patten <jvp@justinvp.com>
Fri, 15 Mar 2019 14:52:31 +0000 (07:52 -0700)
committerStephen Toub <stoub@microsoft.com>
Fri, 15 Mar 2019 14:52:31 +0000 (10:52 -0400)
I was looking over the new Range methods on `Collection<T>` and it occurred to me that for all existing methods, parameter validation is done in the *public* methods before calling the *protected virtual* methods. This changes the new Range methods to do the same.

src/System.Private.CoreLib/shared/System/Collections/ObjectModel/Collection.cs

index 40c8ed8..5d6c9c9 100644 (file)
@@ -116,7 +116,25 @@ namespace System.Collections.ObjectModel
             InsertItem(index, item);
         }
 
-        public void InsertRange(int index, IEnumerable<T> collection) => InsertItemsRange(index, collection);
+        public void InsertRange(int index, IEnumerable<T> collection)
+        {
+            if (items.IsReadOnly)
+            {
+                ThrowHelper.ThrowNotSupportedException(ExceptionResource.NotSupported_ReadOnlyCollection);
+            }
+
+            if (collection == null)
+            {
+                ThrowHelper.ThrowArgumentNullException(ExceptionArgument.list);
+            }
+
+            if ((uint)index > (uint)items.Count)
+            {
+                ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.index, ExceptionResource.ArgumentOutOfRange_ListInsert);
+            }
+
+            InsertItemsRange(index, collection);
+        }
 
         public bool Remove(T item)
         {
@@ -131,9 +149,60 @@ namespace System.Collections.ObjectModel
             return true;
         }
 
-        public void RemoveRange(int index, int count) => RemoveItemsRange(index, count);
+        public void RemoveRange(int index, int count)
+        {
+            if (items.IsReadOnly)
+            {
+                ThrowHelper.ThrowNotSupportedException(ExceptionResource.NotSupported_ReadOnlyCollection);
+            }
+
+            if ((uint)index > (uint)items.Count)
+            {
+                ThrowHelper.ThrowArgumentOutOfRange_IndexException();
+            }
+
+            if (count < 0)
+            {
+                ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.count, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum);
+            }
+
+            if (index > items.Count - count)
+            {
+                ThrowHelper.ThrowArgumentException(ExceptionResource.Argument_InvalidOffLen);
+            }
+
+            RemoveItemsRange(index, count);
+        }
+
+        public void ReplaceRange(int index, int count, IEnumerable<T> collection)
+        {
+            if (items.IsReadOnly)
+            {
+                ThrowHelper.ThrowNotSupportedException(ExceptionResource.NotSupported_ReadOnlyCollection);
+            }
 
-        public void ReplaceRange(int index, int count, IEnumerable<T> collection) => ReplaceItemsRange(index, count, collection);
+            if ((uint)index > (uint)items.Count)
+            {
+                ThrowHelper.ThrowArgumentOutOfRange_IndexException();
+            }
+
+            if (count < 0)
+            {
+                ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.count, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum);
+            }
+
+            if (index > items.Count - count)
+            {
+                ThrowHelper.ThrowArgumentException(ExceptionResource.Argument_InvalidOffLen);
+            }
+
+            if (collection == null)
+            {
+                ThrowHelper.ThrowArgumentNullException(ExceptionArgument.list);
+            }
+
+            ReplaceItemsRange(index, count, collection);
+        }
 
         public void RemoveAt(int index)
         {
@@ -172,21 +241,6 @@ namespace System.Collections.ObjectModel
 
         protected virtual void InsertItemsRange(int index, IEnumerable<T> collection)
         {
-            if (items.IsReadOnly)
-            {
-                ThrowHelper.ThrowNotSupportedException(ExceptionResource.NotSupported_ReadOnlyCollection);
-            }
-
-            if (collection == null)
-            {
-                ThrowHelper.ThrowArgumentNullException(ExceptionArgument.list);
-            }
-
-            if ((uint)index > (uint)items.Count)
-            {
-                ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.index, ExceptionResource.ArgumentOutOfRange_ListInsert);
-            }
-
             if (GetType() == typeof(Collection<T>) && items is List<T> list)
             {
                 list.InsertRange(index, collection);
@@ -202,26 +256,6 @@ namespace System.Collections.ObjectModel
 
         protected virtual void RemoveItemsRange(int index, int count)
         {
-            if (items.IsReadOnly)
-            {
-                ThrowHelper.ThrowNotSupportedException(ExceptionResource.NotSupported_ReadOnlyCollection);
-            }
-
-            if ((uint)index > (uint)items.Count)
-            {
-                ThrowHelper.ThrowArgumentOutOfRange_IndexException();
-            }
-
-            if (count < 0)
-            {
-                ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.count, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum);
-            }
-
-            if (index > items.Count - count)
-            {
-                ThrowHelper.ThrowArgumentException(ExceptionResource.Argument_InvalidOffLen);
-            }
-
             if (GetType() == typeof(Collection<T>) && items is List<T> list)
             {
                 list.RemoveRange(index, count);