views:

416

answers:

6

I thought throwing an exception is good practice to let it bubble back up to the UI or somewhere where you log the exception and notify the user about it.

Why does resharper say it is redundant?

try
{
    File.Open("FileNotFound.txt", FileMode.Open);
}
catch
{
    throw;
}
+16  A: 

Because the above statement has the same behavior as if it were not there. Same as writing:

File.Open("FileNotFound.txt", FileMode.Open);
Otávio Décio
So it is better so write that line of code without the try?
orandov
Yes. Don't catch what you're not going to handle.
John Saunders
@orandov - not necesarily. You may for example catch an IOException and throw a more meaningful (from the users' perspective) exception derived from ApplicationException.
Otávio Décio
Yes. If you're going to handle the exception put it in a try/catch. If you don't care about the exception being thrown, don't put it in a try/catch... I think it just muddles the stack trace when it is actually handled further upstream.
John Weldon
@John Weldon - If you use "throw" instead of "throw ex" the stack trace will stay intact.
Dana Holt
@Twisted: The stack trace stays intact, but if the exception wasn't handled, why catch it?
John Saunders
throwing an exception without doing anything, to which the question is referring, is a performance drain. Either catch it and handle it, or don't catch it. Don't just throw, or throw ex.
Wes P
@John Weldon - Yes, I agree, just pointing out that a throw in a catch does not have to muddle the stack. Sometimes I do a try...catch for logging, log the error, then use throw to keep the stack.
Dana Holt
@ocdecio thanks for the quick response. Everyone else thanks for the clarifications. This always confused me...
orandov
+26  A: 

Because

try {
    File.Open("FileNotFound.txt", FileMode.Open);
} catch {
    throw;
}

is no different than

File.Open("FileNotFound.txt", FileMode.Open);

If the call to File.Open(string, FileMode) fails, then in either sample the exact same exception will find its way up to the UI.

In that catch clause above, you are simply catching and re-throwing an exception without doing anything else, such as logging, rolling back a transaction, wrapping the exception to add additional information to it, or anything at all.

However,

try {
    File.Open("FileNotFound.txt", FileMode.Open);
} catch(Exception ex) {
    GetLogger().LogException(ex);
    throw;
}

would not contain any redundancies and ReSharper should not complain. Likewise,

try {
    File.Open("FileNotFound.txt", FileMode.Open);
} catch(Exception ex) {
    throw new MyApplicationException(
        "I'm sorry, but your preferences file could not be found.", ex);
}

would not be redundant.

Justice
@Justice thanks for the thorough answer with sample code.
orandov
+2  A: 

Because the code in the try is already throwing the exception.

You would only want to catch and re-throw the exception if you are going to do something else in the catch block in addition to re-throwing the exception.

Jumipo
+1  A: 

You have not done any processing in the catch block, just thrown the exception again.

It warns you because there is no point in having that try...catch block there.

Also, another good tip is that "throw ex" will not preserve the stack trace but "throw" will.

Dana Holt
+2  A: 

Because it's redundant.

Jason Watts
Because it's redundant.
Neil N
Since when is stating the question as answer a real answer :)?
orandov
"When it's redundant".
John Saunders
lol, I tried to add the redundancy tag twice.. but alas, it failed.
Neil N
but I guess I can add redundant as well.
Neil N
A: 

It's worth noting that while...

try
{
    DoSomething();
}
catch
{
    throw;
}

...is reduntant, the following is not...

try
{
    DoSomething();
}
catch (Exception ex)
{
    // Generally a very bad idea!
    throw ex;
}

This second code snippet was rife through a codebase I inherited a few projects ago and it has the nasty effect of hiding the original exception's stack trace. Throwing the exception that you just caught in this way means that the top of the stack trace is at the throw level, with no mention of DoSomething or whatever nested method calls actually caused the exception.

Good luck debugging code that does this!

Drew Noakes
R# can be set to warn that the latter is 'probably not intended'.
AakashM

related questions