Method knows too much about methods it's calling

Question

I have a method that I want to be "transactional" in the abstract sense. It calls two methods that happen to do stuff with the database, but this method doesn't know that.

public void DoOperation()
{
    using (var tx = new TransactionScope())
    {
        Method1();
        Method2();

        tc.Complete();
    }
}

public void Method1()
{
    using (var connection = new DbConnectionScope())
    {
        // Write some data here
    }
}

public void Method2()
{
    using (var connection = new DbConnectionScope())
    {
        // Update some data here
    }
}

Because in real terms the TransactionScope means that a database transaction will be used, we have an issue where it could well be promoted to a Distributed Transaction, if we get two different connections from the pool.

I could fix this by wrapping the DoOperation() method in a ConnectionScope:

public void DoOperation()
{
    using (var tx = new TransactionScope())
    using (var connection = new DbConnectionScope())
    {
        Method1();
        Method2();

        tc.Complete();
    }
}

I made DbConnectionScope myself for just such a purpose, so that I don't have to pass connection objects to sub-methods (this is more contrived example than my real issue). I got the idea from this article: http://msdn.microsoft.com/en-us/magazine/cc300805.aspx

However I don't like this workaround as it means DoOperation now has knowledge that the methods it's calling may use a connection (and possibly a different connection each). How could I refactor this to resolve the issue?

One idea I'm thinking of is creating a more general OperationScope, so that when teamed up with a custom Castle Windsor lifestyle I'll write, will mean any component requested of the container with OperationScopeLifetyle will always get the same instance of that component. This does solve the problem because OperationScope is more ambiguous than DbConnectionScope.

Answer 1

I'm seeing conflicting requirements here.

On the one hand, you don't want DoOperation to have any awareness of the fact that a database connection is being used for its sub-operations.

On the other hand, it clearly is aware of this fact because it uses a TransactionScope.

I can sort of understand what you're getting at when you say you want it to be transactional in the abstract sense, but my take on this is that it's virtually impossible (no, scratch that - completely impossible) to describe a transaction in such abstract terms. Let's just say you have a class like this:

class ConvolutedBusinessLogic
{
    public void Splork(MyWidget widget)
    {
        if (widget.Validate())
        {
            widgetRepository.Save(widget);
            widget.LastSaved = DateTime.Now;
            OnSaved(new WidgetSavedEventArgs(widget));
        }
        else
        {
            Log.Error("Could not save MyWidget due to a validation error.");
            SendEmailAlert(new WidgetValidationAlert(widget));
        }
    }
}

This class is doing at least two things that probably can't be rolled back (setting the property of a class and executing an event handler, which might for example cascade-update some controls on a form), and at least two more things that definitely can't be rolled back (appending to a log file somewhere and sending out an e-mail alert).

Perhaps this seems like a contrived example, but that is actually my point; you can't treat a TransactionScope as a "black box". The scope is in fact a dependency like any other; TransactionScope just provides a convenient abstraction for a unit of work that may not always be appropriate because it doesn't actually wrap a database connection and can't predict the future. In particular, it's normally not appropriate when a single logical operation needs to span more than one database connection, whether those connections are to the same database or different ones. It tries to handle this case of course, but as you've already learned, the result is sub-optimal.

The way I see it, you have a few different options:

1. Make explicit the fact that Method1 and Method2 require a connection by having them take a connection parameter, or by refactoring them into a class that takes a connection dependency (constructor or property). This way, the connection becomes part of the contract, so Method1 no longer knows too much - it knows exactly what it's supposed to know according to the design.

2. Accept that your DoOperation method does have an awareness of what Method1 and Method2 do. In fact, there is nothing wrong with this! It's true that you don't want to be relying on implementation details of some future call, but forward dependencies in the abstraction are generally considered OK; it's reverse dependencies you need to be concerned about, like when some class deep in the domain model tries to update a UI control that it has no business knowing about in the first place.

3. Use a more robust Unit of Work pattern (also: here). This is getting to be more popular and it is, by and large, the direction Microsoft has gone in with Linq to SQL and EF (the DataContext/ObjectContext are basically UOW implementations). This sleeves in well with a DI framework and essentially relieves you of the need to worry about when transactions start and end and how the data access has to occur (the term is "persistence ignorance"). This would probably require significant rework of your design, but pound for pound it's going to be the easiest to maintain long-term.

Hope one of those helps you.