What are the benefits of using a 'decorator factory' that decorates objects?

Question

In a project I decided to implement the Decorator pattern.

I have a class Thing with methodA(), and a class AbstractDecorator that inherits from Thing and that all decorators inherit from: ConcreteDecorator1, ConcreteDecorator2, etc. All of them of course override methodA() to add some functionality before delegating to the wrapped Thing. Usual Decorator implementation.

I decided to implement a WrappingFactory (for a lack of a better name): it receives Thing objects and wraps them with a specified decorator. Some of the decorators require a parameter in the constructor, and WrappingFactory takes care of that too. Here it is:

public class WrappingFactory{

    public static void addSomeFunctionality(Thing thing){
        thing = new SomeDecorator(thing);
    }

    public static void addFunctionalityWithParameter(Thing thing, int param){
        thing = new DecoratorWithParameter(thing, param);
    }

    public static void addSomeAwesomeFunctionality(Thing thing){
        thing = new AwesomeDecorator(thing);
    }

}

I did this but actually I don't know why. Does this have benefits as opposed to having the client instantiate decorators directly?

If this has benefits, please explain them to me.

Answer 1

First, MichaelT's comments are spot-on. And there's absolutely no reason to create an AbstractDecorator class.

That said, here is an example of a "decorator factory": the factory determines, based on the input filename, whether or not to add a GZip decoder into the stack of decorated streams.

public static InputStream openFile(File file)
throws IOException
{
    InputStream stream = null;
    try
    {
        stream = new FileInputStream(file);
        stream = new BufferedInputStream(stream);
        if (file.getName().endsWith(".gz"))
            stream = new GZIPInputStream(stream);
        return stream;
    }
    catch (IOException ex)
    {
        closeQuietly(stream);
        throw ex;
    }
}

The other -- main -- reason that this factory is useful is that it properly handles cleanup in case one of the constructors fails. Which is something that (1) many programmers (myself included) won't get right if they make explicit calls, and (2) eliminates boilerplate code.

Answer 2

Assuming your code is C#, then your code is wrong. You are creating the the decorator, but not returning it.

Either, Thing parameter should be ref:

public static void addSomeFunctionality(ref Thing thing){
    thing = new SomeDecorator(thing);
}

Or you return the newly created decorator instance:

public static Thing addSomeFunctionality(Thing thing){
    return new SomeDecorator(thing);
}

First case makes it hard to use on newly created instance, as you have to create temporary variable to ref against. Second is practically useless and I think it violated YAGNI.

But one change might make it much more useful, and that is making the static methods into extension methods.

public static Thing addSomeFunctionality(this Thing thing){
    return new SomeDecorator(thing);
}

This way you can easily chain them:

var finalThing = new BasicThing().addSomeFunctionality().addFunctionalityWithParameter("param");

But if all of the static methods do is call the constructor, then I would go the YAGNI way and don't bother.

Answer 3

As always with the static factory pattern, you get the benefit of being able to easily change creation-implementation.

For example - let's say you have a decorator that takes a type as an argument:

public enum Type{TYPE_A,TYPE_B}

public class ThingWithType implements Thing{
    private Type type;
    private Thing thing;

    public ThingWithType(Thing thing,Type type){
        this.thing=thing;
        this.type=type;
    }
}

public class WrappingFactory{
    public Thing addType(Thing thing,Type type){
        return new ThingWithType(thing,type);
    }
}

Now let's say you want to separate ThingWithType into two classes, one for TYPE_A and one for TYPE_B. Now you can do:

public enum Type{TYPE_A,TYPE_B}

public class ThingWithTypeA implements Thing{
    private Thing thing;

    public ThingWithTypeA(Thing thing){
        this.type=type;
    }
}

public class ThingWithTypeB implements Thing{
    private Thing thing;

    public ThingWithTypeB(Thing thing){
        this.type=type;
    }
}

public class WrappingFactory{
    public Thing addType(Thing thing,Type type){
        switch(type){
            case TYPE_A: return new ThingWithTypeA(thing,type);
            case TYPE_B: return new ThingWithTypeB(thing,type);
            default: throw new Error(String.format("Can't handle type %s",type));
        }
    }
}

And you don't have to alter the actual places where you call WrappingFactory.addType. If you didn't have WrappingFactory, you would have to hunt for places in your code where you wrap Thing with ThingWithType and replace it there.

Edit:

I've read MichaelT's comment and I have to agree. Design patterns are solutions, and you sound like you have a solution and are now looking for a problem.

Answer 4

1. You should not depend on concrete classes. To make sure you don't you could use a factory of some kind.

2. You might want to change how to decorate your object later on, so encapsulating the decoration part would keep you flexible. I think one of the reasons to use decorators is that you might want to use more than one. Otherwise you might simply subclass. And you might change your decorators at some point. Or stack more decorators on top. I'll throw in Liskov Substitution Principle here - you might not even WANT to know how you decorated your object. So better don't depend on concrete decorators.

Therefore I say using a factory to decorate your objects is a good thing. Maybe even a Builder pattern.

Please be aware I always have unit tests in mind, for testability dependencies can be problematic.