Saturday, March 7, 2009

Decorator pattern: alternative to using AOP?

I've wanted to check out some of the Aspect Oriented Programming frameworks out there just to get a feel for how they behave in the code. I understand the paradigm and the vocabulary but I haven't really played around with it. AOP is one of those things I keep promising myself to check out but I always put it off. Now I'm finally sitting down to check out what's around and how it would fit in with my day-to-day.

If you're an AOP framework you're generally implemented in one of two ways; either you're compile-time and the code is "weaved" into the class of choice or you're runtime and a proxy is generated for the class in question. Basically, you're looking to attach some behavior to a class that will happen before a method call, after a method call or when an exception is thrown.

AOP being a tool in my six-demon bag, I have to see how it can improve any of my current projects. Right now, I only have the ubiquitous logging issue that is commonly mentioned in the same breath as AOP. The usual problem is thus; you have some component who's main concern is not logging wonderfully verbose diagnostic information but rather that it builds a list of accounting records for export to a general ledger or it's approximating how late an MBTA train can be before the commuting populace riots. To keep the gods of Truth and Beauty satisfied you must separate these concerns.

My only problem is that I may have already. I hadn't seen it done this way before but I was using the decorator pattern to effectively perform the same work that an AOP framework would. I had fallen into using decorator pattern recently to eliminate inappropriate inheritance (just about any inheritance is inappropriate to me but that's a topic for another post). Think of it this way; the AOP framework is providing a mechanism to intercept method calls and do some magic before or after the a method is executed. The decorator pattern is doing just that. I'll break down the pros and cons for using an AOP framework or using the decorator pattern in a moment but here's a code snippet to demonstrate the thought.

NOTE: For the sake of brevity I'm hoping you can make logical leaps about the types and classes I'm using. They're purely fictional and they're not what's important.

public interface IComicBookRepostitory
{
ComicBookCollection GetComicsBy(SearchParameters searchParams);
}

An IComicBookRepository is simply a repo for comic books. In this case it has a method for retrieving comic books by a set of parameters. The concrete class for the repo looks like the following.

public class ComicBookRepository : IComicBookRepostitory
{
public ComicBookCollection GetComicsBy(SearchParameters searchParams)
{
return new ComicBookCollection();
}
}

This class obviously does nothing of import but that's ok. We can fill this in with whatever we please when we know what that implementation will look like.

I can create a class that will "decorate" any instance of IComicBookRepostitory with error logging. Hence the LoggedComicBookRepository.

public class LoggedComicBookRepository : IComicBookRepostitory
{
private ILog _log;
private IComicBookRepostitory _wrappedRepo;

public LoggedComicBookRepository(ILog log, IComicBookRepostitory wrappedRepo)
{
_log = log;
_wrappedRepo = wrappedRepo;
}

public ComicBookCollection GetComicsBy(SearchParameters searchParams)
{
try
{
_wrappedRepo.GetComicsBy(searchParams);
}
catch (ArgumentException ex)
{
_log.Error(
string.Format("Parameters: Genre {0}, Year {1}", searchParams.Genre, searchParams.Year), ex);
throw;
}
}
}

Notice that it contains an instance of an IComicBookRepostitory. The LoggedComicBookRepository has a constructor that is configured with an ILog and an IComicBookRepostitory. The ILog will capture logged messages and the IComicBookRepostitory is what is being wrapped (decorated per the pattern). Whenever the wrapped instance of the IComicBookRepostitory throws an error it will be captured and the appropriate error logging will occur. Calling GetComicsBy(searchParams) is now completely logged and our concrete ComicBookRepository is none the wiser. Likewise, if we wanted to do more work before or after the method is called for any other reason then we may do so and capture the essence of AOP.

Here's some setup code that will perform comic book retrieval that will be logged.

public class SomeClass
{
public void DoSomething()
{
IComicBookRepostitory repo =
new LoggedComicBookRepository(
log4net.LogManager.GetLogger(GetType(LoggedComicBookRepository)),
new ComicBookRepository());

SearchParameters searchParams = new SearchParameters(Genre.Action, 1995);
ComicBookCollection collection = repo.GetComicsBy(searchParams);
}
}

That setup/configuration code can easily be hidden behind a DI container or something similar but I'm just illustrating how it would generally work.

The advantage to this is that I don't need to bring in a new library with XML configuration, code setup and an API that may not be familiar to other developers. It's also swatting a fly with a bazooka. In the scenario presented above we really don't need to involve a professional AOP solution.

Where AOP makes sense is for a more generic scenario. With the code I've been working with, I want to emit important information about specific method calls and their inputs. If I didn't care for that then an AOP framework would be a far better choice. For example, if I wanted to include code that profiled method execution then I could have a stopwatch that started on method entry and stopped on method exit using an AOP framework. The only thing it may capture about the method is its name. If I tried to hack an AOP framework into my previous example I would have to generate additional code that would provide mapping between method names, their overloads and the class that was responsible with logging each.

So, once again, I've avoided digging into an AOP framework. I know what I need to know and when I find the square hole I'll put the square block of AOP into it.

5 comments:

Elliott said...

Hey Nick!

I'm an AOP guy, myself.

Its much less intrusive, and adheres to "Tell Don't Ask" nicely (kinda).

(Very) Arguably, your example violates SRP, since your Logged Repository now not only serves as a Repository, but also logs. While it doesn't understand the details of how to log, it still has this line

string.Format("Parameters: Genre {0}, Year {1}", searchParams.Genre, searchParams.Year), ex);

Being the purist I am ;) I Like AOP because you'd have gotten rid of that.

While I think that YAGNI has a little weight here, there's a lotta bang for buck with going with AOP. Once the framework is in place, you can add logging, auditing, and whatever you want.

To me, your example added just one more blade to the the(non SRP) swiss army knife, sure it's only got two blades right now, but once it's got 2, why not add a can opener?

http://www.lostechies.com/blogs/derickbailey/archive/2009/02/11/solid-development-principles-in-motivational-pictures.aspx


If you REALLY didn't like AOP,
I'd actually prefer inheritance here actually, because I could write ComicBookRepository that adhered to SRP. Then LoggedComicBookReposity:ComicBookRepository. Then you'd just override GetComicBooksBy and only do the logging, let the base class do the getting. At least then you'd have SRP...

Whatta ya think?

Elliott said...

haaaaaaaaa

Strike what I just said, It was early.

I still like AOP better, but you code does exactly what I was trying to say.

I hadn't had my energy drink yet!
Sorry man!

Nick said...

Not even a problem! I'm psyched that someone actually read my blog! You have done your job :)

Elliott said...

you should briefly consider returning the favor!
;)

Steven said...

I don't agree with Elliott that the decorator violate Single Responsibility Principle (SRP). SRP is about responsibilities of the code and the code of the decorator only logs. It doesn't matter how many interfaces that decorator implement, it only logs. With that argument all decorators will always violate the SRP. The fact that it is statically depends on tje search params doesn't matter either from a SRP perspective. It does make the implementation less useful though, since you'll probably need to create a dozen of logging decorator classes for all reposotories in the system.

So the SRP is not the problem. There are two other SOLID violations. Both the Open/Close Principle (OCP) and Interface Segregation Principle (ISP) are violated.

Since the IComicBookRepostitory contains the GetComicsBy query method, it's very likely more query methods will be added to this interface (the design implies this). Every time a query method is added, the interface has to be updated and all the implementations (real repo + decorators + fakes) have to be broken open. This is a Open/Close violation.

The IComicBookRepostitory currently has one member and does not violate the ISP in its current form. But again, more query methods will be added in the future. When this happens, no single consumer will depend on all query methods. Most consumers will only depend on a single query method, making those methods not cohesive. And this is a ISP violation.

We can fix this by putting each query method in its own class. This prevents the commic book repo interface from ever changing which will fix both the OCP and ISP violations. Furthermore, this allows all repos to be based on the same generic IRepository(Of TEntity) interface or perhaps a IRepository(Of TEntity, TKey).

And here's the clue: when consumers depend on closed generic versions of that interface (such as IRepository(Of CommicBook)) we can define one single generic logging repository decorator that can be wrapped around all repositories. This way there is no need for code weaving or dynamic proxies. We simply wrap a closed generic decorator implementation around a repository implementation.

Not only should repositories get their own generic interface, but this holds for those query objects as well. Imagine a generic IQuery(Of TQueryParams, TResult) for instance with a single "TResult Execute(TQuery)" method. This again allows you to define one single logging decorator that can be applied to all queries in the system.

And it gets even better. Decorators can be written in a tool-agnostic way and are therefore often much easier to read, test and maintain than interceptors or aspects are. Have you ever tried to test an aspect or test business logic in isolation without the aspects being applied? You'll know what I'm talking about. And by applying generic type constraints to those generic decorators, we can conditionally apply them in a way that is natural to developers (again, no framework specific API here) and you get compile-time support out of the box. And since there is no reflection going on, you can have performance that is as good as with code weaving (or even better).

I've written a lot about this type of architecture on my blog. If you're interested, take a look at this post:

http://www.cuttingedge.it/blogs/steven/pivot/entry.php?id=92