views:

136

answers:

6

Hey everyone, I was thinking about passing method blocks around as arguments to helper classes that built in exception handling but it's one of those things that is intuitive and I'd like to submit it for criticism, insight, or advice.

I would like to note up front that this is NOT how I do all of my exception handling, but there are cases where I find this structure more "readable."

For example, I have a scenario where I'm showing a preview image but if that fails (this is a real scenario where I'm previewing images and certain GIF/BMP formats cannot be previewed) it's simply a scenario where I display an alternate image instead of preview. The try/catch code block that looks like this:

  try
  {
        alternatePreviewImage.SetSource(fs);
  }
  catch (Exception ex) {
        requiresSpecialPreview = false;
        previewImage = new BitmapImage(new Uri("Images/NoPreviewAvailable.png", UriKind.Relative));
  }

So I'll leverage a helper class that takes a method parameter to make it look like this:

  if(!ErrorHelper.RunWithSuccessNotify(()=> alternatePreviewImage.SetSource(fs))){
        requiresSpecialPreview = false;
        previewImage = new BitmapImage(new Uri("Images/NoPreviewAvailable.png", UriKind.Relative));                             
  }

The ErrorHelper.RunWithSuccessNotify is quite simple:

public static bool RunWithSuccessNotify(Action code) {
 bool success = true;
 try
 {
  code();
 }
 catch (Exception ex)
 {
  success = false;
 }

 return success;
}

Let me again underscore that it is useful for these low impact scenarios, as well as others where I may be able to suppress the exception:

public static void RunWithErrorSuppression(Action code) {
 try
 {
  code();
 }
 catch (Exception ex)
 {
  // pass
 }
}

The approach could be more detailed too, to allow for capturing the exception:

    public static void ExecuteWithLogging(Action code, Action<Exception> handles) {
        try
        {
            code();
        }
        catch (Exception ex)
        {
            handles(ex);
        }
    }

So what are thoughts on this set of tactics to centralize exception handling? If it is a bad direction, are there specific reasons why it might end up getting me in trouble?

A: 

Hello i don't know if it scale well but for now my team is using it in our measurement software to handle special exception (like communication with some measurement device lost) and restart the measurement in this case.

For now it's working well especially in cases where the important / non repetitive part of the code is in the function (Action parameter) being called and not really in the exception handling.

The best case being that they even could be nested and the main code kept readable.

On the other side you are hiding what exactly your exception handling is doing behind a function name, so it should be simple or clear enough to be understandable just by the name of the function, otherwise you are obfuscating something from the future readers of your code (yourself included)

VirtualBlackFox
+3  A: 

The main problem I would have with this approach is catching Exception is generally considered bad form. If there was some way to specify which type of exception to catch I might buy it, but I believe that has to be specified at compile time. I also wouldn't feel completely comfortable with catching all exceptions and rethrowing the ones that don't match.

Remember, when you catch an exception, you're essentially saying that you can handle the error in some meaningful way. Obviously, the code above can't handle StackOverflowException or MissingMethodException.

Lee
Small detail: You can't catch `StackOverflowException` in recent versions of the framework, unless you threw it yourself.
Joren
You can have a generic version where you specify the expected exception
Vinko Vrsalovic
Good point Vinko, though I still don't see how this solution is more maintainable, expressive, readable or performant. (And yes, I put performant at the end deliberately.)
Lee
A: 

If I've understood what you're trying to do, then it feels horrible to me as a standard error management pattern. That's a warning sign right there, as it took me a while to grasp your idea.

Unless there's a good reason, code should be explicitly available in the method for any maintenance developer to view and modify, not passed around from other locations. Just because a feature exists doesn't mean that it's useful in most situations. In this case, I can't see any benefit from the obfuscation.

BTW, I think you're nailing the corpse to the wall by catching System.Exception. The normal rule is not to catch an exception unless you know what the exception is and/or you need to know the exception detail for some reason.

A commonly-used pattern for recovery in the event of an unknown exception looks something like this:

bool exceptionHappened = true;
try
{
    alternatePreviewImage.SetSource(fs);
    exceptionHappened = false;
}
finally
{
    if ( exceptionHappened )
    {    
        requiresSpecialPreview = false;
        etc;
    }
}
RoadWarrior
How is such a finally { if (expcetionHappened) { .. } } different from catching Exception? Can one throw something which doesn't derive from Exception?
harms
The issue with catching an exception where you don't need to do so is that you then either have to swallow the exception (usually not good) or you have to rethrow it. Catch-rethrow disturbs the exception double-pass mechanism, and therefore gives a miserable debugging experience.
RoadWarrior
A: 

My initial thought is simply that passing complex anonymous delegates around smells bad. There's nothing stopping someone from dumping an anonymous delegate with complex logic into the ErrorHelper methods. Such logic then becomes difficult to test. I would prefer logic to be passed only in an object, which to me seems to be a better understood technique. To me anonymous delegates (and lambdas) are for very simple logic extensibility, like comparison logic. The mechanics of anonymous delegates are powerful and can be used for other things, but with great power comes great responsibility. =)

What you achieve with your technique is the ability to cleanly change the exception handling strategy at runtime. But is this an actual requirement?

gWiz
Lambdas have way more use than simple logic. Think outside the box.
ChaosPandion
Ah, thanks for the instruction. It never occurred to me to think out of the box. I'll start doing that.Hehe, of course lambdas have more use. Does that mean every possible systemic decoupling should be achieved by using lambdas? No way. In most cases, I'd argue that object-oriented dependency injection is still better. Think outside the outside of the box.
gWiz
A: 

Have a look at THIS and maybe whip out the ol' Reflector on it for a bit. Seems to me like the same things are done there but possibly in a more complete way... It seems very powerful...

Kris
A: 

Don't trap all the exception, parametrize the one you want:

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

namespace ProvaException
{
    class Program
    {
        static void Main(string[] args)
        {

            int a = doWithException<int>(divide, 0, typeof(DivideByZeroException), x => 300);
            Console.WriteLine(a);
            Console.ReadKey();
        }

        public static int divide(int b)
        {
            return 10 / b;
        }

        public static T doWithException<T>(Func<T, T> a, T param1, Type exType, Func<Exception, T> handFunction) {
            try
            {
                return a(param1);
            }
            catch(Exception ex) {
                if(exType.Equals(ex.GetType())) {
                     return handFunction(ex);
                }
                else 
                    throw ex;
            }
        }
    }
}

Not very C# like, but it can be useful in same case (when you want to abstract handling exception).

You have to write a different type signature for every type of function, because C# doesn't support currying. Veeeeeeeeery boring. :D

It's not a very elegant solution, but it gives you an hint on how things works in functional languages. It's very fun to learn.

volothamp