Making an atomic transaction out of several operations

Question

I'm looking for a design pattern or suggestions that can help refactor my code into something a bit less repetitive. I have a method that has several sequential steps (10-15) that if any of them fail must record detail about the failure to a log and rollback all previous portions of the transaction. The example is below.

I have thought about taking each try catch and turning it into an individual method but then I have to pass around my undo stack (among several other variables) and I still have to conditionally abort the method and process the undo stack. I've briefly looked into the Momento and Command patterns but both seemed to grow the line count by a fair margin.

EnumResult TransactionThatRollsBack()
{
        var undoLog = new Stack<Action>();
        try
        {
            MoveInventory(inv, src, dest);
            undoLog.Push(() => MoveInventory(inv,dest,src));
        }
        catch (Exception ex)
        {
            RecordError("InventoryError", ex, inv, src, dest);
            Undo(undoLog);
            return EnumResult.FailedInventoryMove;
        }

        try
        {
            NotifyReportingOfMove(inv, user);
            undoLog.Push(() => NotifyReportingOfUnMove(inv, user));
        }
        catch (Exception ex)
        {
            RecordError("NotifyReportingOfMove", ex, inv, user);
            Undo(undoLog);
            return EnumResult.FailedReportingMove;
        }

        try
        {
            AddUserToSweepstakes(user);
            undoLog.Push(() => RemoevUserFromSweepstakes(user));

        }
        catch (Exception ex)
        {
            RecordError("AddUserToSweepstakes", ex, user);
            Undo(undoLog);
            return EnumResult.FailedSweepstakesAdd;
        }

        ...
        return EnumResult.Success
}

Answer 1

You can remove repeated code if you record all your actions for executions and then execute them all at once with some generic exception handling. The idea is below.

Sorry for some possible lapses, didn't write in C# for quite some time. However I hope that you'll find the whole idea to be helpful.

  EnumResult TransactionThatRollsBack()
    {
         var undoLog = new SmartStack();
         undoLog.Push(() => MoveInventory(inv,dest,src), EnumResult.1);
         undoLog.Push(() => NotifyReportingOfUnMove(inv, user), EnumResult.2);
         undoLog.Push(() => RemoevUserFromSweepstakes(user), EnumResult.3);

         undoLog.execute();
    }

    class SmartStack {
       SortedDictionary<Action, EnumResult> actions; 

       void push(Action a, EnumResult r) {
         actions.add(a, r);
       }

       void execute() {
         Stack<Action> completedActions = new Stack<Action>();
         foreach(KeyValuePair<Action, EnumResult> pair in actions) {
             try {
               pair.Key.Invoke();
               completedActions.push(pair.Key);
             }
             catch(Exception ex) {
               for (Action a in completedActions) {
                   Undo(a);
               }
               return pair.Value;
             }
         }
       }
    }

Answer 2

Much of that needs to be there. You could modify things to use a single try-catch though:

    var undoLog = new Stack<Action>();
    var errorContainer = new ErrorContainer{ message = "Unknown error", enumResult = EnumResult.UnknownError };

    try
    {
        errorContainer = GenerateError(EnumResult.FailedInventoryMove, "InventoryError", inv, src, dest);
        MoveInventory(inv, src, dest);
        undoLog.Push(() => MoveInventory(inv,dest,src));

        errorContainer = GenerateError(EnumResult.FailedReportingMove, "NotifyReportingOfMove", inv, user);
        NotifyReportingOfMove(inv, user);
        undoLog.Push(() => NotifyReportingOfUnMove(inv, user));

        ...

        return EnumResult.Success;
    }
    catch (Exception ex)
    {
        Undo(undoLog);
        LogError(errorContainer, ex);
        return errorContainer.enumResult;
    }

I don't know that you gain clarity by going this route, but if you can make the ErrorContainer work for you, it might be an option to reduce total lines of code.

Answer 3

Well I would personally want to cut down on all of that exception handling code.

The easiest way in my mind is to create your own exceptions. Now, I understand you seem to need a message and enum associated with each chunk there so why not make that as a base transactionException and overload each case.

public class TransactionException : Exception
{
    public TransactionException(string message,EnumResult result,Exception innerException) : base(message,innerException) { Result = result; }

    public EnumResult Result { get; private set; }
}

Encapsulates the basic functionality of a transaction and now in specific cases you can do something like:

public class InventoryMoveException : TransactionException
{
    public InventoryMoveException(Exception innerException) : this("InventoryError",innerException) {}
    public InventoryMoveException(string message,Exception innerException) : base(message,EnumResult.FailedInventoryMove,innerException){}
}

and with all that in place you should be able to throw a number of custom exceptions and wrap the whole thing in one catch block that will log the associated message and return the associated enum.