Decorator pattern mess

Question

I'm having a problem figuring out if I'm using the decorator pattern the right way. Let's suppose I'm working on a console application. In this application I have defined a simple IConfigPathProvider interface, which will provide a configuration file path to some class.

public interface IConfigPathProvider
{
    string GetConfigPath();
}

The path is stored in the appSettings section of the console application's app.config file.

public class AppSettingsConfigPathProvider : IConfigPathProvider
{
    public string GetConfigPath()
    {
        return System.Configuration.ConfigurationManager.AppSettings["configPath"];
    }
}

The thing is this path is also encrypted, so...

public class DecryptingConfigPathProvider : IConfigPathProvider
{
    private readonly IConfigPathProvider _provider;
    private readonly IStringDecrypter _decrypter;

    public DecryptingConfigPathProvider(IConfigPathProvider provider, 
        IStringDecrypter decrypter)
    {
        _provider = provider ?? throw new ArgumentNullException(nameof(provider));
        _decrypter = decrypter ?? throw new ArgumentNullException(nameof(decrypter));
    }

    public string GetConfigPath()
    {
        var path = _provider.GetConfigPath();
        //decrypting method of IStringDecrypter interface
        return _decrypter.DecryptString(path);
    }
}

But, wait: it's not over. I have to add a specific portion to the path to get it right.

public class AppendSectionConfigPathProvider : IConfigPathProvider
{
    private readonly IConfigPathProvider _provider;

    public AppendSectionConfigPathProvider(IConfigPathProvider provider)
    {
        _provider = provider ?? throw new ArgumentNullException(nameof(provider));
    }

    public string GetConfigPath()
    {
        var path = _provider.GetConfigPath();

        return System.IO.Path.Combine(
            System.IO.Path.GetDirectoryName(path),
            "section", 
            System.IO.Path.GetFileName(path));
    }
}

And now - why not? - let's add some logging.

public class LoggingConfigPathProvider : IConfigPathProvider
{
    private readonly static ILog _log = 
        LogManager.GetLogger(typeof(LoggingConfigPathProvider));

    private readonly IConfigPathProvider _provider;

    public LoggingConfigPathProvider(IConfigPathProvider provider)
    {
        _provider = provider ?? throw new ArgumentNullException(nameof(provider));
    }

    public string GetConfigPath()
    {
        _log.Info("Getting config path...");
        var path = _provider.GetConfigPath();

        _log.Info("Config path retrieved successfully!");
        return path;
    }
}

Divide et impera

Of course the instant outcome is the separation of concerns, BUT what about the added complexity in instantiating the object? You need to know which decorator comes first and in which order they should be chained, otherwise you'll end up with a buggy IConfigPathProvider.

new LoggingConfigPathProvider(
    new AppendSectionConfigPathProvider(
        new DecryptingConfigPathProvider(
            new AppSettingsConfigPathProvider(), 
            decrypter));

And this is just a simple provider. In a rather complex application you'd likely come across multiple components with multiple references...this could easily led to a maintenance nightmare. Now, is this a well-known drawback or I'm just using this pattern in the wrong way?

We have a similar situation with our code, but we do not think too much of it. As far as I am concerned, wiring up the decorators in the wrong order is a programmer error. Besides, you--as the programmer--need to dictate the correct order given that you are eventially performing a well-defined sequence of operations. — RWRkeSBZ, Sep 24 '18 at 13:11
You might look into builder pattern as an alternative, imho that works better with better readability of code in a situation like this. Also you need to consider, should it be dynamic, i.e. different parts of code can add different provider to get the final object, in that case decorator is better. But it looks like you are using the pattern to create/build the object right away. Builder pattern should fit better in that case. — AD.Net, Sep 24 '18 at 14:34
@AD.Net actually I don't think the builder pattern would be the best fit in a situation like this: 1) the only optional/dynamic argument would be logging, so each and every other decorator has to be chained no matter what. Keep in mind this is just a rather simplistic example, in production systems I may have even 10more mandatory decorators (big messy constructor); 2) if, instead of burdening the builder constructor, I decide to use builder methods just to improve readability in creating the object, I'd end up adding ambiguity (which method should I call first? which later? in which order?) — lucacelenza, Sep 25 '18 at 08:02

score 1 · Answer 1 · answered Sep 24 '18 at 20:12

This is a well-known drawback. The GoF mentions the following liability of the Decorator Pattern.

Lots of little objects. A design that uses Decorator often results in systems composed of lots of little objects that all look alike. The objects differ only in the way they are interconnected, not in their class or in the value of their variables. Although these systems are easy to customize by those who understand them, they can be hard to learn and debug.

score 1 · Answer 2 · answered Oct 10 '18 at 19:30

You're not necessarily correct. Rather than decorating object right away, keep some kind of a decoration shema, validatable, lazy, which can be converted into needed (final, ready-to-use) object by calling, let say, .Build(). Just a code sketch: obj.DecorateWith<Decorator1>().DecorateWith<Decorator2>().DecorateWith(() => new Decorator3(IContainer.Resolve<SomeWhatArgument> ...).Build(). It makes things definitely harder, however as long as decorating is right way to go and your project is indeed big enough to benefit from such a high abstraction, it will solve your problem.

Decorator pattern mess

2 Answers2

Linked