Is Martin Fowler's POEAA implementation of Unit of Work an anti-pattern?

Question

Well from the book POEAA, Martin Fowler introduced this idea of Unit of Work. It works very well if you want to have auto-commit system, in which your domain model uses Unit of work to label itself as new, dirty, removed or clean. Then you only need to call UnitofWork.commit() and all changes of models will be saved. Below is a domain model class with such methods:

public abstract class DomainModel{

    protected void markNew(){
        UnitOfWork.getCurrent().registerNew(this);
    }

    protected void markDirty(){
        UnitOfWork.getCurrent().registerDirty(this);
    }

    protected void markRemoved(){
        UnitOfWork.getCurrent().registerRemoved(this);
    }        

    protected void markClean(){
        UnitOfWork.getCurrent().registerClean(this);
    }
} 

With this implementation, you can mark a domain model as any save state through business logic method:

public class Message extends DomainModel{

    public void updateContent(User user, string content){
        // This method update message content if the the message posted time is not longer than 24 hrs, and the user has permission to update messate content.
        if(!canUpdateContent(user) && timeExpired()) throw new IllegalOperationException("An error occurred, cannot update content.");
        this.content = content;
        markDirty();
    }  
}

At first glance, it looks marvelous, since you dont have to manually call insert, save and delete method on your repository/data mapper. However, I see two problems with this approach:

1. Tight coupling of domain model with Unit of work: This implementation of Unit of Work will make domain models dependent on UnitOfWork class. UnitOfWork has to come from somewhere, the implementation of static class/method is bad. To improve this, we need to switch to dependency injection, and pass an instance of UnitOfWork to the constructor of Domain Model. But this still couples domain model with Unit of work. Also ideally a domain model should only accept parameters for its data fields(ie. Message domain model's constructor should only accept whats relevant to message, such as title, content, dateposted, etc). If it will need to accept a parameter of UnitOfWork, it will pollute the constructor.

2. The domain model now becomes persistent-aware: In modern application design, especially DDD, we strive for persistent-ignorant model. The domain model shouldnt care about whether it is being persisted or not, it should not even care about whether there's persistence layer at all. By having those markNew(), markDirty(), etc methods on domain model, our domain models now have the responsibility of informing the rest of our application that it needs to be persisted. Although it does not handle the persistence logic, the model still is aware of the existence of persistence layer. I am not sure if this is a good idea, to me it seems to have violate the single responsibility principle. There's also an article talking about this: http://blog.sapiensworks.com/post/2014/06/04/Unit-Of-Work-is-the-new-Singleton.aspx/

So what do you think? Does the original Unit of Work pattern described in Martin Fowler violate good OO design principles? If so, do you consider it an antipattern?

Answer 1

From a DDD perspective, this is something you shouldn't do.

DDD contains the following rule:

An application service should only modify one aggregate per transaction.

If you follow this rule, it's clear which aggregate changed during an app service operation. This aggregate then in turn needs to be passed to a repository for saving to the DB:

repository.update(theAggregate);

No other call is required. This defeats the gain from the pattern in the form you describe it.

On the other hand, the pattern you describe introduces a dependency from the domain to the persistence mechanism (depending on the design either a real dependency or just a conceptual dependency). Now this is something you should avoid, because it increases the complexity of your model a lot (not only internally, also for clients).

As a result, you shouldn't use the pattern in this form together with DDD.

Outside of DDD

Having that said, I think the pattern is one of many solutions to a certain problem. That solution has pros and cons, some of which you describe in the question. In some situations, the pattern may be the best trade-off, so

No, this is not an anti-pattern.

Answer 2

To be entirely accurate, there is no one "Martin Fowler's implementation of Unit of Work". In the book he distinguishes between two types of registration of a modified object into a UoW.

Caller registration where only the calling object knows about the UoW and has to mark the (callee) domain object as dirty with it. No anti pattern or bad practice here as far as I can tell.

Object registration where the domain object registers itself with the UoW. Here again there are two options :

For this scheme to work the Unit of Work needs either to be passed to the object or to be in a well-known place. Passing the Unit of Work around is tedious but usually no problem to have it present in some kind of session object.

The code sample is using UnitOfWork.GetCurrent() which is closer to the latter option and admittedly widely considered an anti-pattern today because of the tightly coupled, implicit dependency (Service Locator style).

However, if the first option was chosen, i.e. passing the UoW over to the domain object, and let's assume a Unit of Work abstraction, would it be bad practice ? From a dependency management perspective, clearly not.

Now remains the persistence ignorance aspect. Can we say about an object which can signal another object it's just been edited/created/removed that it is persistence-aware ? Highly debatable. In comparison, if we look at more recent domain object implementations out there, for instance ones in Event Sourcing, we can see that aggregates can be responsible for keeping a list of their own uncommitted changes which is more or less the same idea. Does this violate persistence ignorance ? I don't think so.

Bottom line : the specific code Fowler chose to illustrate one of many UoW possibilities would clearly be considered bad practice now, but much more so with regard to problem #1 you pointed out and not really problem #2. And this doesn't disqualify other implementations he writes about, nor the whole UoW pattern whose change-tracking mechanics are anyway most of the time hidden away in third party library magic (read: ORM) nowadays and not hardcoded as in the book's example.