Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

A problem when using the decorator design pattern

We are currently using the decorator design pattern to perform some caching. So we have a bunch of classes that look something like this:

interface IComponent
{
  object Operation();
  object AnotherOperation();
}
public ConcreteComponentA : IComponent
{
  public object Operation()
  {
    return new object();
  }
  public object AnotherOperation()
  {
    return new object();
  }
}
public ConcreteDecoratorA : IComponent
{
  protected IComponent component;
  public object Operation()
  {
    if(!this.cache.Contains("key")
    {
      this.cache["key"] = this.component.Operation();
    }
    return this.cache["key"];
}

So if a client wanted to use caching they would create a new ConcreteDecoratorA and pass in a ConcreteComponentA to the constructor. The problem we are facing is, imagine that AnotherOperation() requires a call to Operation in order to do it's work. ConcreteComponentA might now look something like this:

public ConcreteComponentA : IComponent
{
  public object Operation()
  {
    return new object();
  }
  public object AnotherOperation()
  {
    object a = this.Operation();
    // Do some other work
    return a;
  }
}

The problem is that when calling Operation() method from within AnotherOperation() method, the decorator implementation will never be called, because obviously the decorator is not in the inheritance hierarchy of ConcreteComponentA.

So have we made a poor design decision somewhere or is this just a limitation of the decorator design pattern that we have to accept?

Note that in my real world example, ConcreteComponentA is a wrapper to a third party system that we do not have control over. We have developed IComponent and a bunch of POCOs that we work with to abstract away that third party system. In this case we have to make two calls to their system in order to get the data required, it's just about where we make those two calls.

like image 341
evan w Avatar asked Oct 04 '10 21:10

evan w


2 Answers

You could create an overload of AnotherOperation which takes the IComponent to be used as a parameter.

public ConcreteComponentA : IComponent
{
  public object Operation()
  {
    return new object();
  }
  public object AnotherOperation()
  {
    return AnotherOperation(this);
  }
  public object AnotherOperation(IComponent comp)
  {
    object a = comp.Operation();
    // Do some other work
    return a;
  }
}

public ConcreteDecoratorA : IComponent
{
  protected IComponent component;
  public object Operation()
  {
    if(!this.cache.Contains("key")
    {
      this.cache["key"] = this.component.Operation();
    }
    return this.cache["key"];
  }
  public object AnotherOperation()
  {
    return this.component.AnotherOperation(this);
  }
}
like image 102
mikemanne Avatar answered Oct 05 '22 02:10

mikemanne


Create a delegate (or an event if you want to support multiple decorators) that allows decorators to manually "override" the Operation method.

public class ConcreteComponentA : IComponent
{
    public event Func<object> OperationOverride;

    public object Operation()
    {
        if (OperationOverride != null)
        {
            return OperationOverride();
        }
        return new object();
    }

    public object AnotherOperation()
    {
        var a = Operation();
        // Do some other work
        return a;
    }
}

In the decorator constructor attempt to cast the component instance into your concrete component type and attach an Operation override delegate.

public class ConcreteDecoratorA : IComponent, IDisposable
{
    protected readonly IComponent component;

    public ConcreteDecoratorA(IComponent component)
    {
        this.component = component;
        AttachOverride();
    }

    public void Dispose()
    {
        DetachOverride();
    }

    private void AttachOverride()
    {
        var wrapper = component as ConcreteComponentA;
        if (wrapper != null)
        {
            wrapper.OperationOverride += Operation;
        }
    }

    private void DetachOverride()
    {
        var wrapper = component as ConcreteComponentA;
        if (wrapper != null)
        {
            wrapper.OperationOverride -= Operation;
        }
    }
}

Use the disposable pattern to ensure that the event is unhooked when the decorator is no longer needed to prevent memory leaks.

like image 28
Nathan Baulch Avatar answered Oct 05 '22 02:10

Nathan Baulch