Proper nullable type checking in C#?

Question

Ok, my actual problem was this: I was implementing an IList<T>. When I got to CopyTo(Array array, int index), this was my solution:

void ICollection.CopyTo(Array array, int index)
{
    // Bounds checking, etc here.
    if (!(array.GetValue(0) is T))
        throw new ArgumentException("Cannot cast to this type of Array.");
    // Handle copying here.
}

This worked in my original code, and still works. But it has a small flaw, which wasn't exposed till I started building tests for it, specifically this one:

public void CopyToObjectArray()
{
    ICollection coll = (ICollection)_list;
    string[] testArray = new string[6];

    coll.CopyTo(testArray, 2);
}

Now, this test should pass. It throws the ArgumentException about not being able to cast. Why? array[0] == null. The is keyword always returns false when checking a variable that is set to null. Now, this is handy for all sorts of reasons, including avoiding null dereferences, etc. What I finally came up with for my type checking was this:

try
{
    T test = (T)array.GetValue(0);
}
catch (InvalidCastException ex)
{
    throw new ArgumentException("Cannot cast to this type of Array.", ex);
}

This isn't exactly elegant, but it works... Is there a better way though?

Answer 1

There is a method on Type specifically for this, try:

if(!typeof(T).IsAssignableFrom(array.GetElementType()))

Answer 2

List<T> uses this:

try
{
    Array.Copy(this._items, 0, array, index, this.Count);
}
catch (ArrayTypeMismatchException)
{
  //throw exception...
}

Answer 3

The only way to be sure is with reflection, but 90% of the time you can avoid the cost of that by using array is T[]. Most people are going to pass a properly typed array in, so that will do. But, you should always provide the code to do the reflection check as well, just in case. Here's what my general boiler-plate looks like (note: I wrote this here, from memory, so this might not compile, but it should give the basic idea):

class MyCollection : ICollection<T> {
   void ICollection<T>.CopyTo(T[] array, int index) {
       // Bounds checking, etc here.
       CopyToImpl(array, index);
   }
   void ICollection.CopyTo(Array array, int index) {
       // Bounds checking, etc here.
       if (array is T[]) { // quick, avoids reflection, but only works if array is typed as exactly T[]
           CopyToImpl((T[])localArray, index);
       } else {
           Type elementType = array.GetType().GetElementType();
           if (!elementType.IsAssignableFrom(typeof(T)) && !typeof(T).IsAssignableFrom(elementType)) {
               throw new Exception();
           }
           CopyToImpl((object[])array, index);
       }
   }
   private void CopyToImpl(object[] array, int index) {
       // array will always have a valid type by this point, and the bounds will be checked
       // Handle the copying here
   }
}

EDIT: Ok, forgot to point something out. A couple answers naively used what, in this code, reads as element.IsAssignableFrom(typeof(T)) only. You should also allow typeof(T).IsAssignableFrom(elementType), as the BCL does, in case a developer knows that all of the values in this specific ICollection are actually of a type S derived from T, and passes an array of type S[]

Answer 4

Here is a little test of try / catch vs. reflection:

object[] obj = new object[] { };
DateTime start = DateTime.Now;

for (int x = 0; x < 1000; x++)
{
    try
    {
        throw new Exception();
    }
    catch (Exception ex) { }
}
DateTime end = DateTime.Now;
Console.WriteLine("Try/Catch: " + (end - start).TotalSeconds.ToString());

start = DateTime.Now;

for (int x = 0; x < 1000; x++)
{
    bool assignable = typeof(int).IsAssignableFrom(obj.GetType().GetElementType());
}
end = DateTime.Now;
Console.WriteLine("IsAssignableFrom: " + (end - start).TotalSeconds.ToString());

The resulting output in Release mode is:

Try/Catch: 1.7501001
IsAssignableFrom: 0

In debug mode:

Try/Catch: 1.8171039
IsAssignableFrom: 0.0010001

Conclusion, just do the reflection check. It's worth it.