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.

  • 7
    Why would you need a factory rather than just pass the object to be decorated into the constructor of the decorator? It sounds like you are over engineering for the sake of over engineering and then wondering why it doesn't have the benefits you seem to perceive that are there. Write the code first, then when you recognize you need a given Pattern to solve a specific problem, then you use it. Don't stick a Decorator or Factory (or heaven forbid a DecoratorFactory) in just because its a Pattern. – user40980 May 29 '14 at 18:22
  • @MichaelT I agree that patterns need to be used only when there's a real need. Thing is I'm a hobbyist and my projects are currently too small to actually need too many of these things (1700 LOC this project). So I'm using them to practice these techniques, even though I don't actually need them in my current projects. – Aviv Cohn May 29 '14 at 18:33
  • 2
    So you are wondering why, when you force yourself to use a pattern that solves a particular problem that it isn't useful when you are not trying to solve that problem? – user40980 May 29 '14 at 18:35
  • @MichaelT I was wondering what are the 'text book' benefits of using a static factory to instantiate decorators, since I saw other guys doing this. – Aviv Cohn May 29 '14 at 18:38
  • 8
    You are partaking in cargo cult programming. Patterns are not meant to be building blocks. They are solutions to particular problems. Their use is when you recognize you have a given problem then you can say "a ha! this can be solved by a FlyWeight" and then you use it. Starting from the Patterns and trying to write software around them before the problem is needed leads to inflexible and contrived designs that resemble things from Architecture Astronauts. – user40980 May 29 '14 at 18:43

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.

  • Thanks for the example. And I believe there is a reason to have an AbstractDecorator superclass for the decorators. Most people consider it part of the pattern because it can hold the inner Component for the subclasses. Just saves the subclasses a line of code. – Aviv Cohn May 29 '14 at 20:33
  • 1
    @Prog If having an AbstractDecorator class only saves the subclasses "a line of code", then something is might be wrong here. The abstract superclass of decorators should be the "empty" decorator, that delegates everything to the decorated object, so that it's subclasses, the actual concrete decorators, will only have to implement the methods they modify. This should save you a lot more than a single line of code per subclass... – Idan Arye May 29 '14 at 22:12
  • @IdanArye Oh yeah, forgot about that. – Aviv Cohn May 29 '14 at 22:15

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.

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.

  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.

  • Please leave a comment if I missed the point or said something stupid, I want to learn – HankTheTank Sep 22 '17 at 10:43

protected by gnat Sep 22 '17 at 7:53

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.