Handling a class that doesn't throw exception in c#

Question

I've got some UI code that looks like this:

try
{
    SomeClass classInstance = new SomeClass(someId);
}
catch (Exception exception)
{
    // Content wasn't created, show a message, stop processing
    return;
}

It seems the try catch was added because the constructor for SomeClass would bomb out if the someId it receives isn't valid, and data couldn't be found in a DB.

Running this code through FXCop recently, it warns against using the general Exception, but all SomeClass does is throw a new Exception with a message to say it failed to initialize.

I guess the problem is that the class constructor should have it's own custom exception, which I could then handle in my UI, but I wonder what else I could do to the code above to handle the exception, that meets FXCop requirements?

Answer 1

FxCop's rule exists because the catch (Exception) block above catches all possible exceptions, including low-level exceptions like StackOverflowException that you probably can't catch in a useful way.

The right approach is definitely to throw a more specific type: either one you've invented, or an existing .NET framework exception type that closely matches your situation. (When in doubt, I normally go for InvalidOperationException.)

Alternatively, you could check the exact exception type when catching it. This won't prevent the FxCop warning, but it should address the underlying problem:

catch (Exception exception)
{
    if (exception.GetType() == typeof(Exception))
    {
        // Content wasn't created, show a message, stop processing
        return;
    }
    else
    {
        // Some other exception type that wasn't thrown from our code -
        //    delegate to a higher-level exception handler
        throw;
    }
}

Answer 2

Using InvalidOperationException in place of throwing Exception sounds like it might be sensible.

Answer 3

You don't need a custom exception; just use one of the dozens that already exist in the framework for given circumstances. If someId is bad, throw an ArgumentException -- that's what it's made for. If something's null that shouldn't be, a NullReferenceException will occur; just let it be thrown. Etc. Throwing a plain Exception is a bit like saying "something went wrong -- read the message for details" rather than "this went wrong".

FxCop is complaining about catch (Exception) because it's too commonly abused to swallow up all exceptions rather than letting them propagate and be handled by code that knows how to do so. You should be able to say what types of exceptions are being thrown and catch those, while letting those you don't recognize make their way up the call stack.

Answer 4

You should fix the class constructor. Throwing Exception is never a good idea.

However, to work around the issue you have temporarily (as this is a horrible, unreliable hack), you could check the message of the exception against the one you're expecting:

catch (Exception exception)
{
    if (exception.Message == "whatever your class sets the message to")
        // Content wasn't created, show a message, stop processing
        return;
    else
        // Any other exception should bubble
        throw;
}

Answer 5

If FXCop doesn't like handling the general Exception (and I tend to agree) then maybe you have access to SomeClass's source code. Modify the constructor to throw an exception that is more specific, e.g. ArgumentOutOfRangeException or some custom exception.

In that case your code would then look as follows:

try
{
   SomeClass classInstance = new SomeClass(someId);
}
catch(ArgumentOutOfRangeException exception)
{
   // Content wasn't created, show a message, stop processing
   return;
}

Answer 6

As many others have said, the constructor should not be throwing a naked Exception. Seeing that the constructor retrieves data from DB and throws based on the result, the best solution is to create your own exception class.

Creating exceptions is super-easy in Visual studio. Just type in Exception and press TAB. It will then create the exception class with required constructors(all four of them). Do not be afraid to create classes that don't do very much, that's what they are designed for.

This is how I would write this class:

public class SomeClass {
    public SomeClass(int someId) {
        if (someId < 0) //validation on the ID, can it be negative?
            throw new ArgumentException("someId", "ID cannot be negative");

        //Perform DB operation
        if (/*DB error - not found*/)
            throw new DataNotFoundException("Cannot find record with ID " + someId);
    }
}

[Serializable]
public class DataNotFoundException : Exception {
    public DataNotFoundException() { }
    public DataNotFoundException(string message) : base(message) { }
    public DataNotFoundException(string message, Exception inner) : base(message, inner) { }
    protected DataNotFoundException(
      System.Runtime.Serialization.SerializationInfo info,
      System.Runtime.Serialization.StreamingContext context)
        : base(info, context) { }
}