Sequential Execution: Orchestrator Pattern

I built a code piece that makes it easy to execute a set of sequential operations, that might depend on the same parameter value (passed across the execution) or might need a particular parameter.
There's also a possibility to manage an error from within the operations, by implementing IReversible interface.

This code example can be copy/pasted into a console app, so you can easily check the functionality.

So far, this is what I've done:

```
class Program
{
static void Main(string[] args)
{
var transferenceInfo = new InterbankTranferenceInfo();

var orchestrator = new Orchestrator(new InternalDebitOperation(transferenceInfo),
new InterbankCreditOperation(),
new CommissionOperation());

orchestrator.Run();
}
}

public class InterbankTranferenceInfo : IParameter
{
public bool InternalDebitDone { get; set; }

public bool InterbankCreditDone { get; set; }

public bool CommissionDone { get; set; }
}

public class InternalDebitOperation : Operation, IOperation
{
public InternalDebitOperation(InterbankTranferenceInfo parameter)
: base(parameter)
{
}

public override InterbankTranferenceInfo Execute()
{
return new InterbankTranferenceInfo() { InternalDebitDone = true };
}
}

public class InterbankCreditOperation : Operation, IOperation
{
public override InterbankTranferenceInfo Execute()
{
Parameter.InterbankCreditDone = true;

return Parameter;
}
}

public class CommissionOperation : Operation, IReversible, IOperation
{
public override InterbankTranferenceInfo Execute()
{
Parameter.CommissionDone = true;

// Uncomment this code to test Reverse operation.
// throw new Exc

First, some structural issues:

-
I think the way you're using generics is completely wrong. The generic parameter T is almost useless IInternalOperation (since you're using IParameter there, not T). And if a class has a method IParameter Execute(IParameter parameter), it means that method should accept any IParameter, which is not what you want.

-
Orchestrator works with IParameter, even if all the operations work with some specific subtype. I think Orchestrator should be generic too.

-
Some of your operations don't work correctly if they are called with one overload of Execute(), the rest don't work correctly when called with the other overload. I think that you should have two different interfaces: one for the initial operation (with the parameterless overload of Execute()) and one for the rest of the operations (with the other overload). And Orchestrator should enforce this by taking a single initial operation and a collection of following operations.

-
I think the marker interface IParameter is useless.

With these modifications, your code could look something like:

public interface IOperationBase
{
    OperationStatus Status { get; set; }
}

public interface IOperation : IOperationBase
{
    void Execute(T parameter);
}

public interface IInitialOperation : IOperationBase
{
    T Execute();
}

public abstract class OperationBase : IOperationBase
{
    public T Parameter { get; protected set; }

    public OperationStatus Status { get; set; }

    protected OperationBase()
    {
        Status = OperationStatus.Pending;
    }
}

public abstract class Operation : OperationBase, IOperation
{
    public void Execute(T parameter)
    {
        Parameter = parameter;
        ExecuteInternal();
    }

    protected abstract void ExecuteInternal();
}

public abstract class InitialOperation : OperationBase, IInitialOperation
{
    protected InitialOperation(T parameter)
    {
        Parameter = parameter;
    }

    public abstract T Execute();
}

public class Orchestrator
{
    public IInitialOperation InitialOperation { get; private set; }

    public IOperation[] Operations { get; private set; }

    public Orchestrator(
        IInitialOperation initialOperation, params IOperation[] operations)
    {
        InitialOperation = initialOperation;
        Operations = operations;
    }

    public T Run()
    {
        // omitted
    }
}

One more thing regarding your implementation: the way you roll back the operations on exception doesn't make much sense to me. When rolling back, you usually:

• Roll back only the operations that actually succeeded (your code will try to roll back even operations that didn't actually run).

• Roll back the operations in reverse order: the last operation that ran first, the first operation last.

• Indicate to the caller that the whole operation failed. Either by rethrowing the exception after the rollback completes, or at least by returning false (and true on success).

StackExchange Code Review Q#25435, answer score: 4