Best Practice for repeated code in different Exception 'catch' blocks

[Artie]

Hi,

Is there a best practice for handling repeated code in Exception catch
blocks?

If I have

try
{
}
catch( VerySpecificException e)
{
// do a;
// do b;
// do c;

// do VerySpecific stuff
}
catch( SpecificException e)
{
// do a;
// do b;
// do c;

// do Specific stuff
}
catch (GeneralException e)
{
// do a;
// do b;
// do c;

// do General stuff
}


and each of these catch blocks does tasks a, b and c, then does their
own specific handling, what is the best way to not have to repeat a, b
and c in each block?

Could I call another method to do a, b, c or is there a more elegant
way of handling this?

Any help much appreciated

Thanks

Artie

 

[Peter Morris]

Option 1: Private method you call
Option 2: Anonymous method you call

 

[Pete Kane]

Hi,

Is there a best practice for handling repeated code in Exception catch
blocks?

If I have

try
{
}
catch( VerySpecificException e)
{
// do a;
// do b;
// do c;

// do VerySpecific stuff
}
catch( SpecificException e)
{
// do a;
// do b;
// do c;

// do Specific stuff
}
catch (GeneralException e)
{
// do a;
// do b;
// do c;

// do General stuff
}


and each of these catch blocks does tasks a, b and c, then does their
own specific handling, what is the best way to not have to repeat a, b
and c in each block?

Could I call another method to do a, b, c or is there a more elegant
way of handling this?

Any help much appreciated

Thanks

Artie

Hi Artie, I use static classes to relay the exception


very briefly

public class MyExceptionHandlers
{
static ShowError(SqlException e)
{
code...
}

static ShowError(DifferentException e)
{
code...
}
}

then in your catch code

MyExceptionHandlers.ShowError(your exception goes here)

HTH

 

[Ignacio Machin ( .NET/ C# MVP )]

Hi,

Is there a best practice for handling repeated code in Exception catch
blocks?

If I have

try
{}

catch( VerySpecificException e)
{
  // do a;
  // do b;
  // do c;

  // do VerySpecific stuff}

catch( SpecificException e)
{
  // do a;
  // do b;
  // do c;

  // do Specific stuff}

catch (GeneralException e)
{
  // do a;
  // do b;
  // do c;

  // do General stuff

}

and each of these catch blocks does tasks a, b and c, then does their
own specific handling, what is the best way to not have to repeat a, b
and c in each block?

Could I call another method to do a, b, c or is there a more elegant
way of handling this?

Any help much appreciated

Thanks

Artie

You mark all your common code, right click and select Refactor/Extract
method

It will create a method with teh neccesary signature and all y ou have
to do is copy it to the other places

 

[Roman Wagner]

With c# 3.0 you can write an Extensionmethod to handle your common
exceptionhandling tasks.

public static DoABAndC(this GeneralException e)
{
//do a
//do b
//do c
}

try
{
}

catch( VerySpecificException e)
{
e.DoABAndC();
// do VerySpecific stuff
}

catch( SpecificException e)
{
e.DoABAndC();
// do Specific stuff
}

catch (GeneralException e)
{
e.DoABAndC();
// do General stuff
}

If the order does not matter. you can write it as follows.

try {
try { //your code
}
catch( VerySpecificException e)
{
// do VerySpecific stuff
throw;
}

catch( SpecificException e)
{
// do Specific stuff
throw;
}

catch (GeneralException e)
{
// do General stuff
throw;
}
}catch(GeneralException ge)
{
ge.DoABAndC();
}

 

[Cor Ligthert[MVP]]

Artie,

It would be rare that after so many years Net that there would be a "Best
Practise" method for a try catch which was not in one method..

Be aware that you also can use a nested try catch

try x
{
try y
{
try z
{
}
catch
{
}
}
catch
{
}
}

But I definitly won't tell that this is better then the other showed
methods.

Cor

 

[Artie]

Hi,

Is there a best practice for handling repeated code in Exception catch
blocks?

If I have

try
{}

catch( VerySpecificException e)
{
  // do a;
  // do b;
  // do c;

  // do VerySpecific stuff}

catch( SpecificException e)
{
  // do a;
  // do b;
  // do c;

  // do Specific stuff}

catch (GeneralException e)
{
  // do a;
  // do b;
  // do c;

  // do General stuff

}

and each of these catch blocks does tasks a, b and c, then does their
own specific handling, what is the best way to not have to repeat a, b
and c in each block?

Could I call another method to do a, b, c or is there a more elegant
way of handling this?

Any help much appreciated

Thanks

Artie

Thanks for all your help guys.

So it looks like refactoring code a, b and c into another method
really is the most elegant way of doing this. Although I liked your
suggestion Roman about re-throwing to another general catch which does
the common tasks, but as you said, the order is most important here,
and would need to come AFTER the exception-specific code.

I suppose my main question now is whether it's better to refactor the
code into another class entirely, like Pete's suggestion, or keep it
in the class I'm in. Need to see how specific this common code is...

Anyway, thanks again for all your help guys - very much appreciated.

Artie

 

[Pavel Minaev]

Option 1: Private method you call
Option 2: Anonymous method you call

To clarify option 2 (which is probably the one I'd go for if your
catch-block code is small and localized), it's something along the
lines of:

Action abc = delegate
{
// do a;
// do b;
// do c;
};

try
{
}
catch( VerySpecificException e)
{
abc();
// do VerySpecific stuff
}
catch( SpecificException e)
{
abc();
// do Specific stuff
}
catch (GeneralException e)
{
abc();
// do General stuff
}

The main advantage of this approach is that you have access to all the
arguments and local variables of your method inside abc with no need
to do explicit forwarding - you'd only need to do that for the
exception object, if you need it there.

 

[Pavel Minaev]

To clarify option 2 (which is probably the one I'd go for if your
catch-block code is small and localized), it's something along the
lines of:

[...code example snipped...]

The main advantage of this approach is that you have access to all the
arguments and local variables of your method inside abc with no need
to do explicit forwarding - you'd only need to do that for the
exception object, if you need it there.

A nit to pick, if I may: I think the _main_ advantage here is that the  
repeated code is placed within the method that uses it.  I.e. it's a code  
maintenance issue, if anything.

Agreed - I just assumed that the general principle of "make everything
as local as possible" is obvious and does not even deserve an explicit
mention so I went ahead and gave some practical benefits stemming
from it.

But yes, it alone is worth it, even if local variable capture isn't
actually used.

 

[Artie]

To clarify option 2 (which is probably the one I'd go for if your
catch-block code is small and localized), it's something along the
lines of:

  Action abc = delegate
  {
    // do a;
    // do b;
    // do c;
  };

  try
  {
  }
  catch( VerySpecificException e)
  {
    abc();
    // do VerySpecific stuff
  }
  catch( SpecificException e)
  {
    abc();
    // do Specific stuff
  }
  catch (GeneralException e)
  {
    abc();
    // do General stuff
  }

The main advantage of this approach is that you have access to all the
arguments and local variables of your method inside abc with no need
to do explicit forwarding - you'd only need to do that for the
exception object, if you need it there.

Hi Pavel,

Thanks for your input - after yesterday's comments I'd thought that
just calling another 'normal' method would be best, but now I can see
the advantages of calling an anonymous method. I get the repeated
code in once place, but I also don't need to pass it all the
parameters that I'd need (except maybe the Exception itself).

So, I like the idea of anonymous methods - I take it this is what a
'delegate' is, or is used for, but I can't find a simple example on
the internet that works! Can you point me in the right direction
please?

Also, am I right in saying that you'd define the delegate itself
actually WITHIN the method that has multiple catch blocks, to localise
it as much as possible?

Artie

 

[Mythran]

If a, b, and c parts do NOT need to be ran BEFORE the extra exception
handling, you can put them all in the finally block of the try...catch
statements.

try {
} catch (VerySpecificException ex) {
} catch (SpecificException ex) {
} catch (Exception ex) {
} finally {
// do a, b, then c.
}


If they have to happen BEFORE the extra exception handling (the following is
a short but complete Program.cs console application):

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace ConsoleUI
{
class Program
{
static void Main(string[] args)
{
int i = 1;

try {
switch (i) {
case 0: throw new VerySpecificException();
case 1: throw new SpecificException();
default: throw new GeneralException();
}
} catch (GeneralException ex) {
// do a, b, c.

if (ex is VerySpecificException) {
DisplayException((VerySpecificException) ex);
} else if (ex is SpecificException) {
DisplayException((SpecificException) ex);
} else {
DisplayException(ex);
}
}
}

private static void DisplayException(VerySpecificException ex) {
Console.WriteLine("Very Specific Information: {0}", ex.Message);
}

private static void DisplayException(SpecificException ex) {
Console.WriteLine("Specific Information: {0}", ex.Message);
}

private static void DisplayException(GeneralException ex) {
Console.WriteLine("General Information: {0}", ex.Message);
}
}

public class VerySpecificException : SpecificException { }
public class SpecificException : GeneralException { }
public class GeneralException : Exception { }
}

HTH,
Mythran

 

[Artie]

If a, b, and c parts do NOT need to be ran BEFORE the extra exception
handling, you can put them all in the finally block of the try...catch
statements.

try {} catch (VerySpecificException ex) {
} catch (SpecificException ex) {
} catch (Exception ex) {
} finally {

    // do a, b, then c.

}

If they have to happen BEFORE the extra exception handling (the followingis
a short but complete Program.cs console application):

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace ConsoleUI
{
    class Program
    {
        static void Main(string[] args)
        {
            int i = 1;

            try {
                switch (i) {
                    case 0: throw new VerySpecificException();
                    case 1: throw new SpecificException();
                    default: throw new GeneralException();
                }
            } catch (GeneralException ex) {
                // do a, b, c.

                if (ex is VerySpecificException) {
                    DisplayException((VerySpecificException) ex);
                } else if (ex is SpecificException) {
                    DisplayException((SpecificException) ex);
                } else {
                    DisplayException(ex);
                }
            }
        }

        private static void DisplayException(VerySpecificException ex) {
            Console.WriteLine("Very Specific Information: {0}", ex.Message);
        }

        private static void DisplayException(SpecificException ex) {
            Console.WriteLine("Specific Information: {0}", ex..Message);
        }

        private static void DisplayException(GeneralException ex){
            Console.WriteLine("General Information: {0}", ex.Message);
        }
    }

    public class VerySpecificException : SpecificException { }
    public class SpecificException : GeneralException { }
    public class GeneralException : Exception { }

}

HTH,
Mythran

Hi Mythran and thanks for your response.

There's a pre-requisite to your method, and that is that the
Exceptions inherit from each other as you've indicated. I'm not sure
this will always be the case, so I feel this solution won't work in
all scenarios. (Unless I just catch Exception or maybe
ApplicationException, and ensure all my Exceptions sub-class those,
which I suppose they probably would...)

I was probably inadvertently slightly misleading in my original post
that the Exceptions WOULD inherit from each other, but I'd prefer a
solution that can handle unrelated exceptions.

Thanks

Artie

 

[Mythran]

Hi Mythran and thanks for your response.

There's a pre-requisite to your method, and that is that the
Exceptions inherit from each other as you've indicated. I'm not sure
this will always be the case, so I feel this solution won't work in
all scenarios. (Unless I just catch Exception or maybe
ApplicationException, and ensure all my Exceptions sub-class those,
which I suppose they probably would...)

I was probably inadvertently slightly misleading in my original post
that the Exceptions WOULD inherit from each other, but I'd prefer a
solution that can handle unrelated exceptions.

Thanks

Artie

Well, Artie, you are correct but you can also use this same code to handle
exceptions that do NOT inherit from each other (although, all exceptions
inherit from the Exception base class) so if you catch Exception instead of
GeneralException, you can do the same thing using the code I posted since
'ex' will always be an instance of a class that is or inherits from
System.Exception.

HTH,
Mythran

 

[Artie]

As long as the code you're repeating really is specific to that one method  
in which the catch clauses appear.  There are still some advantages to  
using named methods, even today.  


For better or worse, the keyword "delegate" is used two different ways in 
C#.  The first way is to declare a delegate type, which is C#'s function  
pointer type.  The second way is to declare an anonymous method.  The 
compiler figures out the difference by the syntax used, since the two  
usages are very different.

Because an anonymous method has no name (duh), the only way you can call  
it is to immediately assign the anonymous method to some delegate  
variable.  This can either be a local variable (as in Pavel's example),or  
a method argument (commonly seen in LINQ and various methods for the  
collection classes in .NET).

MSDN has lots of examples and discussion on the topic of delegate types  
and anonymous methods, so that'd be a good place for you to start.


There's probably no point in doing it any other way.  You can't "capture"  
the local variables in that method unless the anonymous method is declared  
there too.  And if the anonymous method is generalized so that it can be  
declared outside the method where it's used, at that point it might as  
well be a named method.

Note that there were other very good suggestions in response to your  
question.  Which suggestion is really the best approach depends entirely  
on your actual situation, which we don't really know.  If you have some 
generalized handling that can be reused in other methods, an anonymous  
method probably isn't the right approach; the extension method approach,  
or even just a regular named method, is likely to be more useful in that  
case.

But for code that you want to reuse, but which is always _only_ ever going  
to be reused in a single method, I think the anonymous method approach  
works quite nicely.

Pete

Thanks again for your details response, Pete.

In the end, I decided that the scope of the repeated functionality
really was just inside THIS method, so I put the anonymous method in
here, and I agree, it works nicely...in this case.

I've reproduced the method below, in case anyone else finds it useful.

In my case, I DID want to forward the Exception, to log its message,
but when I tried using Action/delegate with the <Exception> part, I
couldn't get it to work. I'm assuming this is possible for cases
where nothing needs to be forwarded to the anonymous method?

For better or worse, the keyword "delegate" is used two different ways inC#

This was the bit that really confused me - most examples I found
seemed to be related to the function pointer case, and very few
showing examples like Pavel's concise usage.

Thanks again for all your help guys - very much appreciated.

Artie

Example:

public static void DoStuff()
{
int a = 5;

Action<Exception> abc = delegate(Exception e)
{
// do a;
// do b;
// do c;

MessageBox.Show(string.Format("In delegate, a = {0}", a));
MessageBox.Show(e.Message);
};

try
{
int throwEx = 1;

if (throwEx.Equals(1))
{
throw new Exception();
}
else if (throwEx.Equals(2))
{
throw new NewException();
}
}
catch (NewException e)
{
// specific stuff
a = 1;

abc(e);
}
catch (Exception e)
{
// specific stuff
a = 2;

abc(e);
}
}

 

[Artie]

[...]
In my case, I DID want to forward the Exception, to log its message,
but when I tried using Action/delegate with the <Exception> part, I
couldn't get it to work.  I'm assuming this is possible for cases
where nothing needs to be forwarded to the anonymous method?

Sorry, I don't understand the question.  In particular, you write "...but  
when I tried using Action/delegate with the <Exception> part, I couldn't  
get it to work".  Yet, the code you posted does use "Exception" as the  
type parameter for the Action<T> delegate type, and should work just fine 
(indeed, your implication is that it does ).

What was it that you _couldn't_ get to work?

Pete

Hi Pete,

Yeah sorry, wee typo there - I meant to say:

So, I tried a separate example from the one I posted where I DIDN'T
want to forward the exception, but I couldn't get Action/delegate to
work when I wasn't forwarding anything.

Artie