views:

142

answers:

5

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.

A: 

Since you have control over both levels (ConcreteComponentA and ConcreteDecoratorA), you can have them hand notes back and forth:

interface IComponent 
{
  Action<object> myNotify;
  object Operation(); object AnotherOperation(); 
} 

public ConcreteComponentA : IComponent
{
  public Action<object> myNotify = null;
  public object Operation()
  {
    object result = new object();
    if (myNotify != null)
    {
      myNotify(result);
    }
    return result;
  }

  public object AnotherOperation()
  {
    return Operation();
  }
}

public ConcreteDecoratorA : IComponent
{
  public ConcreteDecoratorA(IComponent target)
  {
    component = target;
    target.myNotify = notifyMe;
  }
  protected IComponent component;
  protected notifyMe(object source)
  {
    this.cache["key"] = source;
  }

  public Action<object> myNotify = null;
  public object Operation()
  {
    if(!this.cache.Contains("key")
    {
      return component.Operation();
    }
    return this.cache["key"];
  }
  public object AnotherOperation()
  {

  }
}
David B
I see what this example is doing and its a nice bit of code too :) But.. unless I am wrong isn't the original problem that Evs wants ConcreteComponentA.AnotherOperation to call through to ConcreteDecoratorA.Operation, but it cannot. This code still has the same problem doesn't it - ConcreteComponentA.AnotherOperation doesn't call through to the decorator.
Matt Roberts
+1  A: 

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.

Nathan Baulch
+1  A: 

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);
  }
}
mikemanne
A: 

I prefer to use inheritance rather than encapsulation to do my caching, this way, the cached value will use the caching method because it's virtual:

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


public CachingComponentA : ConcreteComponentA
{
     public override object Operation()
     {
         if(!this.cache.Contains("key")
         {
            this.cache["key"] = base.Operation();
         }
         return this.cache["key"];
     }
}

Then when you're using a decorator object, this.Operation() WILL use the decorator class.

Neal Tibrewala
Yes fair enough, however in our case we are converting what is returned from the service into our own domain level POCOs. If the system behind our interface were to be switched out, we would need to Add ConcreteComponentB and then we would have to recreate CachingComponentA as CachingComponentB. That's where encapsulation is nice, we only have a single caching class no matter how many concrete implementations we have.
Evs
+2  A: 

Self-calls are the limitation of decorator design pattern, that's true. The only way to intercept base component self-calls without having to modify it or add any additional infrastructure is inheritance. So if you don't like solutions from above and you still want to have the flexibility which decorator gives you (possibility of having any number and any order of decorators), you can look for an implementation of dynamic proxy that generates subtypes (i.e. Unity Interception, Castle Dynamic Proxy).

A.