CODEWITHSUNDEEP
exceptiontry-catchresharperrethrow
orandov
orandov

Reputation: 3286

Why does resharper say 'Catch clause with single 'throw' statement is redundant'?

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;
}

Upvotes: 20

Views: 6586

Answers (6)

Drew Noakes
Drew Noakes

Reputation: 311255

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!

Upvotes: 1

TWA
TWA

Reputation: 12816

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.

Upvotes: 1

Jason Watts
Jason Watts

Reputation: 1038

Because it's redundant.

Upvotes: 5

Jumipo
Jumipo

Reputation: 246

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.

Upvotes: 4

yfeldblum
yfeldblum

Reputation: 65445

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.

Upvotes: 57

Otávio Décio
Otávio Décio

Reputation: 74290

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

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

Upvotes: 17

Related Questions