Can a scope block with the "using" keyword react to exceptions?

Question

Hi everyone,

I have the need to do some logging within my code. I'm required to use an internal company-developed library to record some information. Here's how it works.

Recorder recorder = Recorder.StartTiming();
DoSomeWork();
recorder.Stop();  // Writes some diagnostic information.

To ensure that Stop() is always called, I created a wrapper class that allows a clean "using" block.

using (RecorderWrapper recorderWrapper = new RecorderWrapper)  // Automatically calls Recorder.StartTiming() under the covers
{
   DoSomeWork();
}  // When the recorderWrapper goes out of scope, the 'using' statement calls recorderWrapper.Dispose() automatically - which calls recorder.Stop() under the covers

it's worked well so far. However, there's a change my company is requiring, that would look something like this on the original code:

Recorder recorder = Recorder.StartTiming();
try
{
   DoSomeWork();
}
catch (Exception ex)
{
   recorder.ReportFailure(ex);  // Write out some exception details associated with this "transaction"
}
recorder.Stop();  // Writes some diagnostic information.

I'd like to avoid try/catches in all my 'using' scope blocks with RecorderWrapper. Is there a way I can accomodate the "ReportFailure()" call and still leverage the 'using' scope block?

Specifically, I want everyone on my team to "fall into a pit of success", i.e. make it easy to do the right thing. To me, this means making it really hard to forget to call recorder.Stop() or forget the try/catch.

Thanks!

Answer 1

You could copy the pattern used by TransactionScope, and write a wrapper that must be actively completed - if you don't call Complete(), then the Dispose() method (which gets called either way) assumes an exception and does your handling code:

using(Recorder recorder = Recorder.StartTiming()) {
    DoSomeWork();
    recorder.Complete();
}

Personally, though, I'd stick with try/catch - it is clearer for maintainers in the future - and it provides access to the Exception.

Answer 2

No, a using block is only syntactic sugar for a try/finally block. It doesn't deal with try/catch. At that point you're going to be left with handling it yourself since it looks like you need the exception for logging purposes.

Answer 3

A using block is effectively a try/finally block that calls dispose on the object in question.

So, this:

using(a = new A())
{
    a.Act();
}

is (i think, exactly) equivalent to this:

a = new A();
try
{
    a.Act();
}
finally
{
    a.Dispose();
}

And you can tack your catches onto the end of the try block.

Edit:

As an alternative to Rob's solution:

Recorder recorder = Recorder.StartNew()
try
{
    DoSomeWork();
}
catch (Exception ex)
{
    recorder.ReportFailure(ex);
}
finally
{
    recorder.Dispose();
}

Answer 4

Don't add another level of indirection. If you need to catch the Exception, use try..catch..finally and call Dispose() in the finally block.

Answer 5

You might be able to create a method on the recorder to hide this:

public void Record(Action act)
{
    try
    {
        this.StartTiming();
        act();
    }
    catch(Exception ex)
    {
        this.ReportFailure(ex);
    }
    finally
    {
        this.Stop();
    }
}

So your example would then just be:

recorder.Record(DoSomeWork);

Answer 6

You could always try something like:

Edit by 280Z28: I'm using a static StartNew() method here similar to Stopwatch.StartNew(). Make your Recorder class IDisposable, and call Stop() from Dispose(). I don't think it gets any more clear than this.

using (Recorder recorder = Recorder.StartNew())
{
    try
    {
        DoSomeWork();
    }
    catch (Exception ex)
    {
        recorder.ReportFailure(ex);
    }
}

Answer 7

You could continue to use the RecorderWrapper you have, but add a TryExecuting method that accepts a lambda of what you want to happen add runs it in a try/catch block. eg:

using (RecorderWrapper recorderWrapper = new RecorderWrapper)  // Automatically calls Recorder.StartTiming() under the covers
{
    recorderWrapper.TryExecuting(() => DoSomeWork());
}

Inside RecorderWrapper:

public void TryExecuting(Action work)
{
    try { work(); }
    catch(Exception ex) { this.ReportFailure(ex); }
}

Answer 8

Oops, I hadn't noticed that a new instance of Recorder was being created by StartTiming. I've updated the code to account for this. The Wrap function now no longer takes a Recorder parameter but instead passes the recorder it creates as an argument to the action delegate passed in by the caller so that the caller can make use of it if needed.

Hmmm, I've needed to do something very similar to this pattern, lambdas, the Action delegate and closures make it easy:

First define a class to do the wrapping:

public static class RecorderScope
{
   public static void Wrap(Action<Recorder> action)
   {
      Recorder recorder = Recorder.StartTiming();
      try
      {
         action(recorder);
      }
      catch(Exception exception)
      {
         recorder.ReportFailure(exception);
      }
      finally
      {
         recorder.Stop();
      }
   }
}

Now, use like so:

RecorderScope.Wrap(
   (recorder) =>
   {
      // note, the recorder is passed in here so you can use it if needed -
      // if you never need it you can remove it from the Wrap function.
      DoSomeWork();
   });

One question though - is it really desired that the catch handler swallows the exception without rethrowing it? This would usually be a bad practice.

BTW, I'll throw in an addition to this pattern which can be useful. Although, it doesn't sound like it applies to what you're doing in this instance: Ever wanted to do something like the above where you want to wrap some code with a set of startup actions and completion actions but you also need to be able to code some specific exception handling code. Well, if you change the Wrap function to also take an Action delegate and constrain T to Exception, then you've got a wrapper which allows user to specify the exception type to catch, and the code to execute to handle it, e.g.:

public static class RecorderScope
{
   public static void Wrap(Action<Recorder> action, 
      Action<Recorder, T1> exHandler1)
      where T1: Exception
   {
      Recorder recorder = Recorder.StartTiming();
      try
      {
         action(recorder);
      }
      catch(T1 ex1)
      {
         exHandler1(recorder, ex1);
      }
      finally
      {
         recorder.Stop();
      }
   }
}

To use.. (Note you have to specify the type of exception, as it obviously cannot be inferred. Which is what you want):

RecorderScope.Wrap(
   (recorder) =>
   {
      DoSomeWork();
   },
   (recorder, MyException ex) =>
   {
      recorder.ReportFailure(exception);
   });

You can then extend this pattern by providing multiple overloads of the Wrap function which take more than one exception handler delegate. Usually five overloads will be sufficient - it's pretty unusual for you to need to catch more than five different types of exceptions at once.