oven Comments on software development

Template method + command pattern can be useful when you don’t have AOP

When working on a web service for a project, we quickly discovered that the service implementation class was starting to take on a bit too much responsibility. Before we execute each call, we need to check that the user's session token is valid and open a session to the database.  Then we can do the actual work, and finally commit or roll back the session as required. The class started to look a bit like this:

public class Service {
	public User GetUserById(int id, Guid sessionToken) {
		Validate(sessionToken);
		OpenSession();
		try {
			User result = dao.RetrieveUserById(id);
			Commit();
			return result;
		} catch (Exception ex) {
			Rollback();
			LogAndRethrow(ex);
		} finally {
			CloseSession();
		}
	}

	public string StoreDocument(Document document, byte[] file, Guid sessionToken) {
		Validate(sessionToken);
		OpenSession();
		try {
			ValidateDocument(document);
			AssignDocumentNumber(document);
			StoreDocumentFile(file);
			dao.StoreDocument(document);
			Commit();
			return document.DocumentNumber;
		} catch (Exception ex) {
			Rollback();
			LogAndRethrow(ex);
		} finally {
			CloseSession();
		}
	}
}

Only with a lot of methods, showing a lot of duplication with respects to session handling and error handling. Now, the first thing you might think of is aspect oriented programming. But is there another way? How would you design a solution that removed that duplication if you didn't have AOP?

Also, we noticed that some service operations do basically nothing except retrieve something from the database, while others do quite a bit of work. The example above shows that if you want to retrieve information about a user for an admin screen, you basically get the user by its user id, and that's that. However, to store a new document, you need to validate the contents of the new document, store the actual document file to the hard drive, assign a new document number from a sequence, and so on. All this should be divided into small, crisp methods that do just one thing. If you leave all these supporting methods in the service class, things start to get ugly pretty fast. What you need is a good separation of concerns, so that the service class can concentrate on one thing, and each service operation gets its own place to live, so that they can concentrate on their things.

AOP will only solve the duplication problem, but it will not solve the separation of concerns, so we decided not to go with that. Instead, we decided to remove the duplication by using a template method, and separate the concerns by creating a command object for each service operation. The template method looks a bit like this:

public class Service {
	private T ExecuteOperation<T>(Operation<T> operation, Guid sessionToken) {
		Validate(sessionToken);
		OpenSession();
		try {
			return operation.Execute();
			Commit();
		} catch (Exception ex) {
			LogAndRethrow(ex);
		} finally {
			CloseSession();
		}
	}
}

And the operation looks a bit like this:

public abstract class Operation<T> {
	protected Dao dao;
	public Operation(Dao dao) {
		this.dao = dao;
	}
	public abstract T Execute();
}

Now, a simple operation like getting a user is fairly small. The extra typing involved by creating a class and overriding the Execute() method and so on seems a bit much:

public class GetUserByIdOperation : Operation<User> {
	private int id;
	public GetUserByIdOperation(Dao dao, int id) : base(dao) {
		this.id = id;
	}

	public override User Execute() {
		return dao.GetUserById(id);
	}
}

But when the operation is a bit more complex, you start to see the benefits:

public class StoreDocumentOperation : Operation<string> {
	private Document document;
	private byte[] file;

	public StoreDocumentOperation(Dao dao, Document document, byte[] file) : base(dao) {
		this.document = document;
		this.file = file;
	}

	public override string Execute() {
		ValidateDocument();
		AssignDocumentNumber();
		StoreDocumentFile();
		dao.StoreDocument(document);
		Commit();
		return document.DocumentNumber;
	}

	private void ValidateDocument() {...}
	private void AssignDocumentNumber() {...}
	private void StoreDocumentFile() {...}
}

Now we have the supporting methods nicely grouped in one place, together with the actual operation they support. Also, we don't have to keep passing the document and file around, because they are now fields.

The original service methods are now reduced quite a bit:

public class Service {
	public User GetUserById(int id, Guid sessionToken) {
		return ExecuteOperation(new GetUserByIdOperation(id, dao), sessionToken);
	}

	public string StoreDocument(Document document, byte[] file, Guid sessionToken) {
		return ExecuteOperation(new StoreDocumentOperation(document, file, dao), sessionToken);
	}

	private T ExecuteOperation<T>(Operation<T> operation, Guid sessionToken) {...}
}

So now we don't have much duplication, the service class now has a lot less responsibility and the cohesion is a lot better, since the service operations and their support methods are now nicely grouped into little classes.