views:

50

answers:

3

I recently came across a code written by a fellow programmer in which he had a try catch statement inside a catch!

Please forgive my inability to paste the actual code, but what he did was something similar to this

try
{
 //ABC Operation
}
catch (ArgumentException ae)
{
   try
   {
      //XYZ Operation
   }
   catch (IndexOutOfRangeException ioe)
   {
      //Something
   }
}

I personally feel that it is one of the poorest code I have ever seen! On a scale of 1 to 10...how soon do you think I should go and give him a piece of my brain? Or am I over-reacting?

EDIT: What he is actually doing is that in the catch, he is performing some other operations which can/should be done when the initial try fails. My problem is with having a clean code and maintainability. Delegating the exception from the first catch to a different function or the calling function would be OK, but adding more code which may or may not throw an exception into the first catch, is what I felt was not good. I try to avoid multiple stacked "if-loop" statements, I found this equally bad.

  • IvarD
A: 

Without knowing what the code does it's impossible to say. But it's not unusual to do this.

e.g. if you have to clear up resources whilst handling exceptions, that clear-up code itself may have the capability to throw exceptions.

Brian Agnew
+2  A: 

Why is that bad? It's no different conceptually than:

void TrySomething() {
   try {


   } catch (ArgumentException) {
        HandleTrySomethingFailure();
   }
}

void HandleTrySomethingFailure() {
    try {

    } catch (IndexOutOfRangeException) {

    }
}

Before you go over there and give him a piece of your brain (try the parietal lobe, it's particularly offensive) , what exactly are you going to say to him? How will you answer the proverbial "why?"

What's even more ironic is that when the jitter inlines this code, it will look exactly like your example.

-Oisin

x0n
I totally agree. If you feel there is something wrong with his code and are willing to confront him about it, you better have an excellent alternative ready to present.
ChrisNel52
A: 

Here is a case :

try{
    //Dangerous Operation
} catch (AnyException ae) {
    try {
        //Do rollback which can fail
    } catch (RollbackFailedException rfe) {
        //Log that
    }
} finally {
    try {
        //close connection but it may fail too
    } catch (IOException ioe) {
        //Log that
    }
}

It's about the same thing as @x0n said. You might need to handle exception while try to close resources or while you're trying to resolve a situation brought by another exception.

Colin Hebert