Search code examples
c#entity-frameworkdesign-patternstry-catchusing

Refactor class to avoid writing same pattern in every method


I have a class with a lot of static methods for managing database operations. All methods follow this pattern:

try
{
   using(var trans = new TransactionScope())
   {
      using(var context = new DBContext())
      {
         . . . // method body
      }
      trans.Complete();
   }
}
catch(Excepcion ex)
{
   Log.Error(ex.Message);
   if (ex.InnerException != null)
      Log.Error(ex.InnerException.Message);
}

How can I refactor my code so that it is not needed to write this structure in every method?

EDIT for implementing Jon's response.

public static T TransactionalOperation<T>(Func<DBContext, T> databaseAction)
{
   T retVal = default(T);
   try
   {
      using (var trans = new TransactionScope())
      {
         using (var context = new DBContext())
         {
            if (databaseAction != null)
               retVal = databaseAction(context);
         }
         trans.Complete();
   }
   catch (Exception ex)
   {
      Log.Error(ex.ToString());
   }
   return retVal;
}
public static void TransactionalOperation(Action<DBContext> databaseAction)
{
   TransactionalOperation(context =>
      {
         databaseAction(context);
         return string.Empty;
      });
}

And used like this:

public static string GetUserLanguage(string owner)
{
   return
      TransactionalOperator(context => context.clients.Single(c => c.Id == owner).Language);
}    

Solution

  • It sounds like you should pass "method body" in as a delegate:

    public void GiveMeAProperName(Action<DBContext> databaseAction)
    {
        try
        {
           using(var trans = new TransactionScope())
           {
              using(var context = new DBContext())
              {
                 . . . // method body
              }
              trans.Complete();
           }
        }
        catch(Exception ex)
        {
           Log.Error(ex.Message);
           if (ex.InnerException != null)
              Log.Error(ex.InnerException.Message);
        }
    }
    

    Then you can call it with:

    GiveMeAProperName(context =>
    {
        // Do stuff with your context here
    });
    

    As noted in comments, you might want to have an overload of:

    public void GiveMeAProperName<T>(Func<DBContext, T> databaseAction)
    

    so that you can return a value. You can easily write your Action overload to just delegate to that:

    public void GiveMeAProperName(Action<DBContext> databaseAction)
    {
        GiveMeAProperName(context =>
        {
            databaseAction(context);
            return "ignored";
        }
    }
    

    I would strongly advise different exception handling though:

    • You're only logging the message. Why do you think the stack trace is unimportant? Just log the whole exception - that will include nested exceptions etc
    • You're effectively ignoring the failure afterwards. I'd personally rip out the try/catch entirely, in most cases... catch exceptions at a very high level, not at the level of doing database operations.