views:

129

answers:

8

Will both catch blocks be hit?

try
{
    DoSomething();
}
catch (AuthenticationException e)
{
    throw;
}
catch (Exception e)
{
    throw new AuthenticationException("Hello ");
}
+7  A: 

It's valid (in that it will compile, build and run), but it's not good practice.

Catching the general exception only to rethrow a specific exception will cause problems. If nothing else you are destroying the original exception call stack.

In response to your edit, both catch blocks won't be hit.

If AuthenticationException is raised the first block will be executed, for any other exception the second block will be executed.

ChrisF
I'd also like to add that you really shouldn't even catch the exception if all you do is immediately throw it again. Unless you're actually _handling_ the exception in some way, don't catch it. Let whatever else is up the stack catch it, since it's going to anyway.
David
@David: but logging an re-throwing could be a good practice. Let's assume the code is artificial.
Henk Holterman
@Henk: I agree that the code in the question is probably just for the sake of example. Just figured it would be good to add to the overall answer for posterity. Even with logging, though, one should weigh the necessity of it. I recently had a debate with a co-worker where he insisted that it should always be caught right away so that you have everything you need to log, and I argued that this is only valid when catching the default exception (ideally rare). Specific and custom (domain) exceptions should carry that information with them up the stack.
David
If I don't catch it then it will be handled by the second catch block which I don't want.... right? That is my problem. I needed to present a generic message to the client for all exceptions except those of type authentication exception.
zachary
@zachary - in that case you should look at handling all unhandled exceptions at the top most level in your program. There are questions on that here on Stack Overflow.
ChrisF
A: 

Yes if you have different kinds of exceptions. (2 exceptions)

No if you wish that the first block will arrive in the second. (1 exception)

Arcturus
A: 

No. Both catch blocks will not be hit.

If DoSomething throws an AuthenticationException, then it will be caught and rethrown.

If DoSomething throws any other exception, a new AuthenticationException will be thrown with the message "Hello".

villecoder
+3  A: 

Only one exception block will be hit. And they go in order, so if DoSomething() throws an AuthenticationException, the first catch block will run.

That said, I'd probably not use try/catches here at all if all you're doing is rethrowing the exceptions. This is not a good practice. At the very least, make sure you add the original exception in your second catch as the InnerException of the AuthenticationException you're creating:

catch(Exception e) 
{
    throw new AuthenticationException(e.Message, e);
}
Anna Lear
A: 

If DoSomething() throws an AuthenticationException then

catch (AuthenticationException e)

will be used. For all other types of exceptions,

catch (Exception e)

But you shouldn't throw a new AuthenticationException inside the second catch.

mathieu
+2  A: 

This code will throw an AutheniticationException if DoSomething throws anything. It will be the same exception if DoSomething throws AuthenticationException, or a new exception in any other case.

A side note - its not really good practise:

  1. You loose all details of an exception which is not AuthenticationException
  2. Why would you throw an AuthenticationException here, if the underlying code thinks there is something else wrong? A code smell for me.
Grzenio
A: 

The second block will not catch the rethrown exception from the first block.

Femaref
A: 

One benefit I can see for catching and rethrowing an exception would be to convey the message, "The requested operation did not succeed, but the system state is essentially as it was before the operation was attempted." While catching all exceptions in an inner routine and hoping that none of them represent a problem which should cause the main program to terminate is somewhat icky, I'm not sure what alternative design is better. One could litter the code with:

  If Not Integer.TryParse(inputString, inputVar) Then
    Throw New MyApp.FileLoadException("Whatever")
  EndIf

but it really seems more natural to just use Integer.Parse and catch any exception that occurs. Catching and recasting general exceptions within a small domain where their expected causes are known is far less evil than swallowing general exceptions at a higher level.

supercat