Is it bad programming style to have a single, maybe common, generic exception?

Question

Hi, so in my program I have parts where I use try catch blocks like this

try
{
  DirectoryInfo dirInfo = new DirectoryInfo(someString); 
 //I don't know if that directory exists
 //I don't know if that string is valid path string... it could be anything

 //Some operations here
}
catch(Exception iDontCareWhyItFailed)
{
  //Didn't work? great... we will say: somethings wrong, try again/next one
}

Of course I probably could do checks to see if the string is valid path (regex), then I would check if directory exists, then I could catch various exceptions to see why my routine failed and give more info... But in my program it's not really necessary. Now I just really need to know if this is acceptable, and what would a pro say/think about that. Thanks a lot for attention.

Answer 1

You shouldn't use exceptions for flow control, because it hurts performance and exceptions are not designed for that purpose. Instead, you should test the Exists property to check that the directory exists

Answer 2

You should do the validation whenever possible. Exceptions are expensive and should be avoided where possible.

Doing checks for the path and other validation doesn't necessarily obviate the need for exception handling and try blocks. Exceptions happen all the time (directory doesn't exist, network isn't connected) it's unexpected, unforeseeable exceptions that try/catch blocks are designed to handle.

Answer 3

• write "mainline" code to handle expected cases.
• write exception handling code to... wait for it... handle exceptional cases. That's why it's called "exception handling code". :-)

If a mainline, expected, everyday case is that the path doesn't exist, then write mainline code that checks whether the path exists. If an unexpected, bizarre, exceptional circumstance is that the file exists but has been locked by another user, write an exception handler that handles that exception.

Answer 4

It really depends on how you're going to handle any of those error conditions.

If in any case you're going to exit and just print the Exception, and your users are familiar with reading and understanding that kind of output, then it's totally fine, and you'd be wasting time and code to handle it more specifically.

On the other hand, if you end up having a bunch of if tests within your exception handling code to do different things based on what exception occurred, then that's pretty clear evidence that you should handle different exceptions differently.

And if your users are not "techies" and will want more helpful error messages than exception dumps, then you should do more custom error handling yourself. Even if they are techies, they would likely appreciate clearer error messages.

There's no right answer. It really depends on your target user experience.

Answer 5

I would not recommend a catch-all type exception handler. Instead, you should lookup DirectoryInfo and the types of exceptions it can throw. Then you should try to avoid as many as possible before you call the constructor and only catch those exceptions that you anticipate. So my code would look like:

try
{
    if ( string.IsNullOrEmpty( someString ) )
        //do something or throw an exception
    if ( someString.Length > 248 )
        //do something or throw an exception

    DirectoryInfo dirInfo = new DirectoryInfo(someString); 

    If ( !dirInfo.Exists )
        //do something or throw an exception

    //path exists and is valid
    //Some operations here
}
catch(SecurityException)
{
  //Didn't work? great... we will say: somethings wrong, try again/next one
}
catch(ArgumentException)
{
  //Didn't work? great... we will say: somethings wrong, try again/next one
}

Answer 6

If you truly don't care why it failed, and your program can't reasonably do anything to recover, then it's OK to do something like this; I'd rather maintain something like this than code that uses 3 try/catch blocks to do the same thing but without adding any additional value. I do like the name of the exception that communicates it isn't of importance, that's better than catching a variable named ex.

Some suggestions, however:

1. If you're going to "catch and ignore", be more explicit about which Exception types are OK to ignore. If you know you can ignore FileNotFoundException, or any class of IOException, then just catch that.

2. If you're going to catch a generic Exception, consider logging the exception somewhere. Your current approach could be a maintenance nightmare if a logical defect exists in your try block. For instance, say you code an off-by-one error with regards to an array index... your current code will swallow that and provide no indication that something more important than "directory does not exist" occurred.

Answer 7

I'm guilty of this kind of lazy coding, as well, usually when delivering quick turnaround on 'hot' features. The problem is that cleaning up the lazy code becomes a low priority, given the new stack of inevitably 'hot' feature requests, which means this kind of lazy code accumulates. These lazy Exception catch blocks then lead to swallowing genuinely unexpected problems and suddenly your application state has mutated in an unexpected way. I'm disciplining myself to not do this, because writing lazy code is like using a credit card. It lets you deliver things now that you would have had to deliver later, but it's the compounding that gets you when it's time to pay it back.

Answer 8

Even if you do checks yourself (it's a valid path, the path exists, you're allowed to access it, it's not locked, etc.), you still need to have the exception handling. Although it seems unlikely, there's no reason that the conditions couldn't change from the time you check them to the time you actually access the path.

As long as you need the exception handling in the first place, why do redundant checks? The OS is going have to do them all again anyway, and odds are you aren't going to get them right anyway.

Don't worry about the cost of exceptions. Compared to the cost of doing I/O, the cost of an exception is negligible.

That said, it's not good practice to catch all exceptions without doing anything. As Seth said, either catch only IOException or log the exception. If you don't, that part of the code will be really hard to debug.