SOLID Architecture and the Decorator Pattern

My last post was on the Single Responsibility Principle and the repository pattern. I received a comment back that my implementation would be cleaner if I had used the decorator pattern. This would allow for easy composition of data flow without creating additional dependencies on particular data stores.

This really set my mind a twitter. Could I do that? Why hadn’t I done that already? With this in mind I set out to look deeper into the decorator.

What is the decorator pattern?

The decorator pattern is a simple way of enabling a chain of behaviour that is determined by composition, not by inheritance or configuration inside an object.

Let’s look at building a cookie without decorators.

public class Cookie
{
	public bool HasChocolateChips { get; set; }
	public bool ContainsNuts { get; set; }

	public Cookie(bool hasChocolateChips, bool containsNuts)
	{
		HasChocolateChips = hasChocolateChips;
		ContainsNuts = containsNuts;
	}

	public decimal GetPrice()
	{
		return 1.00 + (HasChocolateChips ? 0.25 : 0.00)
			+ (ContainsNuts ? 0.50 : 0.00);
	}
}

Simple enough. But as we add more and more options to the cookie, the code gets uglier and uglier. Eventually we need to do something because this doesn’t scale.

Enter the decorator pattern.

public interface ICookie
{
	decimal GetPrice();
}

public class Cookie : ICookie
{
	public Cookie()
	{ }

	public decimal GetPrice()
	{
		return 1.00;
	}
}

public class ChocolateChipDecorator : ICookie
{
	private ICookie _cookie;

	public class ChocolateChipDecorator(ICookie cookie)
	{
		_cookie = cookie;
	}

	public decimal GetPrice()
	{
		return _cookie.GetPrice() + 0.25;
	}
}

public class NutsDecorator : ICookie
{
	private ICookie _cookie;

	public class NutsDecorator(ICookie cookie)
	{
		_cookie = cookie;
	}

	public decimal GetPrice()
	{
		return _cookie.GetPrice() + 0.50;
	}
}

Wow. That’s a lot more code. It’s also gone from 1 class to 3 classes and an interface. It sounds like a maintenance nightmare, but if we are a cookie manufacturer we might have 50 different options that can be composed in many different ways so it might end up being a more scalable solution. Let’s pretend it is.

How do we make different cookies?

new Cookie();
new NutsDecorator(new Cookie());
new NutsDecorator(new ChocolateChipDecorator(new Cookie()));
new ChocolateChipDecorator(new NutsDecorator(new Cookie()));

Yeah, I just threw up in my mouth a little bit.

In fact, if we had 50 of these we would need some sort of builder class in order to handle this, because otherwise it’s just too gross.

But what else can we do? What if we had a decorator that at its base made the cookie twice as expensive?

public class OrganicFlourDecorator : ICookie
{
	private ICookie _cookie;

	public class OrganicFlourDecorator(ICookie cookie)
	{
		_cookie = cookie;
	}

	public decimal GetPrice()
	{
		return _cookie.GetPrice() * 2;
	}
}

Hmmmmmm. Something isn’t right there.

new OrganicFlourDecorator(new ChocolateChipDecorator(new Cookie())).GetPrice();
new ChocolateChipDecorator(new OrganicFlourDecorator(new Cookie())).GetPrice();

The contents of each cookie are exactly the same, but the cost of the first is $2.50. The cost of the second is $2.25. Who likes bugs? Who likes the kinds of bugs that take hours to debug and end up being stupid?

Of course, this is a bit of a straw man argument. Clearly an organic flour cookie should be a different type of base cookie. But what about a different size of cookie? Wouldn’t that logically be a multiplier since you need a different amount of mixins?

Another problem here comes with this possibility:

Cookie cookie = null;
new NutsDecorator(cookie).GetPrice();

This is a null reference exception waiting to happen unless you start polluting all of your potential method chains with null checks. This isn’t going to be caught at compile time either, meaning your users will be the first ones to experience the joy of this bug.

This whole thing is a little silly though. I mean, these are just simple examples. If it fails for simple examples that I created with failure and silliness in mind, it should reasonably work for implementing the repository pattern if I do it in a smart way, right?

public interface IArticleRepository
{
	List<Article> GetArticles();
	void SaveArticles(List<Article> articles);
}

public abstract class RepositoryBase : IArticleRepository
{
	private IArticleRepository _baseRepository;

	public RepositoryBase(IArticleRepository articleRepository)
	{
		_baseRepository = articleRepostory;
	}

	public List<Article> GetArticles()
	{
		List<Article> articles = null;

		if (_baseRepository != null)
		{
			articles == _baseRepository.GetArticles();
		}

		return articles ?? LoadArticles();
	}

	protected abstract List<Article> LoadArticles(); 
}

public class SqlRepository : RepositoryBase
{
	public SqlRepository(IArticleRepository baseRepository)
		: base(baseRepository)
	{ }

	protected List<Article> LoadArticles()
	{
		// Load em from SQL.
	}
}

public class CacheRepository : RepositoryBase
{
	public CacheRepository(IArticleRepository baseRepository)
		: base(baseRepository)
	{ }

	protected List<Article> LoadArticles()
	{
		// Load em from cache.
	}
}

Now you can have a nice easy chain:

IArticleRepository repo = new SqlRepository(new CacheRepository(null));

It will check the cache for articles and if it doesn’t find them, will load them from the SQL database. We’ve even made an abstract base so each implementation doesn’t need to check for null on a base repository. It seems fairly light and easy to understand.

But this only works if you don’t have expiry on your cache. What if the objects in your cache expiry, and you need to reload them from your database the next time they are requested? Well, LoadArticles() in the SqlRepository needs to account for this. But because we only want them saved back in a CacheRepository and not necessarily a MongoRepository we now need to check to see what type IArticleRepository truly is in order to know how we are going to handle it.

Longer chains get even worse.

Long story short, I really looked at the decorator pattern as a possibility. I read that comment and like a lot of developers would do, I thought, “What if I’m totally wrong on this?” I wasn’t too familiar with the decorator pattern so I read up on it. I did not look for reasons it has problems.

In fact, I imagine that most of these conclusions are well known shortfalls with the pattern that people know well. If not I hope I’ve been able to give you something to think about.

I’m more concerned with making my code readable and maintainable than implementing patterns.

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s