views:

55

answers:

4

I have a method doSomething() that has one try catch block, within which I call another method.

    public void doSomething()  
    {  
       try
        {
            doSomethingElse()
        }
        catch
        {
        }

    } 

In that other method doSomethingElse() I dont have any try catch block. I am depending on the main method's try-catch to handle the exception. If there are any exceptions in doSomethingElse() they will be bubbled up to the method doSomething's try catch block.

Is there anything wrong with this approach?

Thanks for your time.

+1  A: 

This is ok, but you forget about ; after doSomethingElse()

Svisstack
SO doesn't have a compiler so I didn't think it'd care over here :)
+2  A: 

This is perfectly legitimate.

Let exceptions bubble up to where you can/know what to do with them.

It is good practice not to litter your code with try/catch blocks that don't add anything.

However, having empty catch blocks is bad practice (though I am assuming the code you have posted is a simplified version omitting the catch block code).

Oded
yes I have omitted the catch block for simplicity. Thanks.
+1  A: 

Nothing wrong with that.

As others have said, don't swallow exceptions. It may also be a good idea not to throw a new exception; rather, just catch (Exception) and throw it. Of course, it should also be noted to try and be specific with exceptions, and not just use the default Exception.

Harry
Thanks. In doSomethingElse(); I am not using the throw statement either. Since it gets called in the doSomething(); method, I expect it to get caught in the try-catch block of the doSomething() method.
Yeah that's fine because you don't have a try/catch in doSomethingElse. The exceptions get swallowed when you want to propagate an error from one try/catch up to a try/catch in a method lower in the stack but you don't throw it at all. Then the problems occur ;)
Harry
A: 

It's smelly. Catching an exception requires that you restore the program state, as though doSomethingElse() was never called. It very much depends on what the method does but it is generally pretty unusual to write a method without any side effects whatsoever. If it has any then those side effects need to be canceled. Only doSomethingElse() can do so, the doSomething() method is powerless to guess how many of those side effects were actually executed. And can thus not reliably restore state.

This is especially the case when you catch all exceptions. You'll grab the nasty ones as well. Like AccessViolation or FatalExecutionEngineError, exceptions that leave the CLR itself in a unknown state, you can never restore that. What happens next is quite unpredictable. You'll get a flurry of additional exceptions only if you're lucky. Diagnosing them is impossible, you threw away valuable info.

Hans Passant