views:

521

answers:

10

What are your thoughts on code that looks like this:

public void doSomething()
{
    try
    {
       // actual code goes here
    }
    catch (Exception ex)
    {
        throw;
    }
}

The problem I see is the actual error is not handled, just throwing the exception in a different place. I find it more difficult to debug because i don't get a line number where the actual problem is.

So my question is why would this be good?

---- EDIT ----

From the answers it looks like most people are saying it's pointless to do this with no custom or specific exceptions being caught. That's what i wanted comments on, when no specific exception is being caught. I can see the point of actually doing something with a caught exception, just not the way this code is.

A: 

I've seen instances where generic exceptions are caught like this and then re-packed in a custom Exception Object.

The difference between that and what you're saying is that those custom Exception objects hold MORE information about the actual exception that happened, not less.

Justin Niessner
+11  A: 

Depending on what quality you are looking at it is not throwing the exception in a different place. "throw" without a target rethrows the exception which is very different from throwing an exception. Primarily a rethrow does not reset the stack trace.

In this particular sample, the catch is pointless because it doesn't do anything. The exception is happily rethrown and it's almost as if the try/catch didn't exist.

JaredPar
I've seen code like this before. Normally it's inserted during debugging, so someone can put a breakpoint on the 'throw'.
Joel Coehoorn
@Joel, gotcha. A better solution may to use VS to break on throw. But it's not nearly as fine grained as it could be :(
JaredPar
The advantage over a breakpoint is that you could log details of a non-critical error during unit testing which is often very useful!
Jon Cage
@Jon agreed. But in this example the code simply rethrows and does nothing.
JaredPar
+2  A: 

I think the construction should be used for handling the exceptions you know you will be throwing inside your code; if other exception is raised, then just rethrow.

Take into account that throw; is different than throw ex;

throw ex will truncate the stack to the new point of throwing, losing valuable info about the exception.

public void doSomething()
{
    try
    {
       // actual code goes here
    }
    catch (EspecificException ex)
    {
        HandleException(ex);
    }
    catch (Exception ex)
    {
        throw;
    }
}
Jhonny D. Cano -Leftware-
In your example the catch (Exception ex) isn't necessary. Simply not handling Exception will have the same effect.
Jon B
A: 

Well for starters I'd simply do

catch
{
   throw;
}

but basically if you were trapping multiple types of exceptions you may want to handle some locally and others back up the stack.

e.g.

catch(SQLException sex) //haha
{
   DoStuff(sex);
}
catch
{
   throw;
}
Eoin Campbell
+1  A: 

Doing something like that is fairly meaningless, and in general I try not to go down the road of doing meaningless things ;)

For the most part, I believe in catching specific types of exceptions that you know how to handle, even if that only means creating your own exception with more information and using the caught exception as the InnerException.

Adam Robinson
A: 

Depends on what you mean by "looks like this", and if there is nothing else in the catch block but a rethrow... if that's the case the try catch is pointless, except, as you say, to obfuscate where the exception occurred. But if you need to do something right there, where the error occurred, but wish to handle the exception furthur up the stack, this might be appropriate. But then, the catch would be for the specific exception you are handl;ing, not for any Exception

Charles Bretana
+2  A: 

It wouldn't be, ideally the catch block would do some handling, and then rethrow, e.g.,

try
{
    //do something
}
catch (Exception ex)
{
    DoSomething(ex); //handle the exception
    throw;
}

Of course the re-throw will be useful if you want to do some further handling in the upper tiers of the code.

Jon Limjap
+1  A: 

Sometimes this is appropriate - when you're going to handle the exception higher up in the call stack. However, you'd need to do something in that catch block other than just re-throw for it to make sense, e.g. log the error:

public void doSomething()
{
    try
    {
       // actual code goes here
    }
    catch (Exception ex)
    {
        LogException (ex);  // Log error...
        throw;
    }
}
Joe Albahari
A: 

I don't think just rethrowing the error would be useful. Unless you don't really care about the error in the first place.

I think it would be better to actually do something in the catch.

You can check the MSDN Exception Handling Guide.

aintnoprophet
A: 

Generally having exception handling blocks that don't do anything isn't good at all, for the simple reason that it prevents the .Net Virtual Machine from inlining your methods when performance optimising your code.

For a full article on why see "Release IS NOT Debug: 64bit Optimizations and C# Method Inlining in Release Build Call Stacks" by Scott Hanselman

Martin Brown