views:

79

answers:

4

I am writing an application in C# that requires me to create an Array object on the fly from some dimensions the user passes in. The Array.CreateInstance() method can throw (by last count) 6 different exceptions that I would want to handle. For each exception I would want to inform the user by a simple MessageBox.Show() and a message tailored to the exceptional circumstance. What I do not want to do is catch the general Exception type, because it is a best practice to not do so. I would try catching ArgumentException or something more specific, but the only common superclass to all of the exceptions is Exception.

Bottom Line: I am trying to figure out the best way to handle having so many different exceptions, and what would be an efficient and, more importantly, maintainable solution.

try
{
    data = Array.CreateInstance(TypeHelper.StringToType(cbDataType.SelectedItem.ToString()), dimensions);
}
catch (OutOfMemoryException) { }
catch (NullReferenceException) { }
catch (NotSupportedException) { }
catch (ArgumentNullException) { }
catch (ArgumentOutOfRangeException) { }
catch (ArgumentException) { }
+9  A: 

Of that list there are only 4 exceptions I would consider catching:

  • NotSupportedException
  • ArgumentNullException
  • ArgumentOutOfRangeException
  • ArgumentException

The other two you should never catch, and as of the later CLR's you can't catch an OOM situation (consider MemoryFailPoint if you need to find out).

Delving deeper into Array.CreateInstance, we see why each of those four would be thrown:

  • NotImplementedException: the type you gave it can't be an array, or is an open generic. Since you are pulling these data types from a fixed list, you should know a priori that these are valid types. I would argue against handling this exception.
  • ArgumentNullException: you should be certain all of the arguments you pass are not null, thus this will never happen and you should not handle this exception.
  • ArgumentOutOfRangeException: one of the lengths is less than 0, which you can test a priori, thus you should not handle this exception.
  • ArgumentException: Thrown if the type is invalid (you've already made sure it is valid) or if there aren't enough lengths, which you can test a priori.

So, my suggested code would be:

// code prior to this point ensures cbDataType only has correct types
// and dimensions has at least 1 dimension and is all greater than or equal to 1
data = Array.CreateInstance(
    TypeHelper.StringToType(cbDataType.SelectedItem.ToString()),
    dimensions);

In summary, I would not handle any exceptions since you should be able to prevent all of them from occurring and you shouldn't care about the instances where you can't possibly handle the exception.

sixlettervariables
+1 argument exceptions are usage exceptions and should never be caught; usage must be guaranteed by caller. If you use code contracts to declare and enforce usage constraints, it will, by default, throw an uncatchable ContractException, which is further reason not to catch usage exceptions, since the pattern will not work in all cases.
Dan Bryant
A: 

I suppose you do some reflection on the base Exception type to attempt to fetch a specific message:

// somewhere ...
public static IDictionary<Type, string> exceptionMessages;

// in your method ...
try { ... }
catch( Exception ex ) {        
    var exType = ex.GetType();
    if( exceptionMessages.ContainsKey(exType) ) {
        MessageBox.Show( exceptionMessages[exType] );
    }
    else {
        throw ex;
    }
}

however, if you need to do anything besides select a string message, you'll need to do what you have posted in your question, and handle each one separately.

although, I suppose you could also create a Dictionary of Types to Action<> delegates... not sure if that's a code smell, though. ;)

dave thieben
+2  A: 

Looking at the documentation for Array.CreateInstance it seems that most of those exceptions are thrown due to violated preconditions, which are things you can check for before calling the method. For example, the ArgumentNullException can be prevented by making sure the arguments you pass are not null, and the NotSupportedException can be prevented by making sure the type you request is supported. Since you seem to have a combobox containing the type to use, this should be quite straightforward.

The only exception it seems you can't prevent is the OutOfMemoryException which arguably you shouldn't try to catch anyway.

Lee
A: 

The obvious answer here is to validate the parameters before you pass them into the Array.CreateInstance method that way you avoid most of those exceptions before they even happen.

However, if there are multiple exception types you want to catch then you are going to have to use reflection to simplify the code. Unfortunately (or maybe it is fortunate) catch blocks are not counted among the many C# constructs that can share scope blocks.

try
{
}
catch (Exception caught)
{
    Type[] types = 
      { 
        typeof(OutOfMemoryException), 
        typeof(NullReferenceException) 
        // Continue adding exceptions to be filtered here.
      };
    if (types.Contains(caught.GetType()))
    {
        // Handle accordingly.
    }
    else
    {
        throw; // Rethrow the exception and preserve stack trace.
    }
}
Brian Gideon
This is cleaner using `(caught is OutOfMemoryException || caught is NullReferenceException || ... )`, which the IDE will let you split over multiple lines for better readability. This pattern is useful when multiple exceptions can be thrown, but these particular exceptions can't/shouldn't be handled.
Dan Bryant