decorators and mixins pass the class they are changing

[dylnhdsn]

In the current implementation, when you write a decorator like...

var MyDecorator = function(base) {
  return { /* stuff */  };
};

...the base argument is the base of the instance's constructor. Based on the extend functionality, I expected that base would be the class being decorated. This would be useful when you want to use a decorator that might be overriding an existing method that you still want.

This is how I expected to be able to use it...

var MyClass = Fiber.extend(function(base) {
  return {
    reallyCoolMethod: function() { /* cool stuff */ }
  };
});

var MyDecorator = function(base) {
  return {
    reallyCoolMethod: function() {
      // more betterer cool stuff
      base.reallyCoolMethod.apply(this, arguments); // or preferably - base.reallyCoolMethod()
    }
  };
}

[Iheartweb]

Good idea, but this is not what decorators do. They're meant to to alter the instance without altering other objects. See http://en.m.wikipedia.org/wiki/Decorator_pattern

[dylnhdsn]

It would still be only altering the instance, but you would have access to the instance original methods. This is something described in the decorator pattern, but not available in Fiber. An example from that wiki article show that.

// the first concrete decorator which adds vertical scrollbar functionality
class VerticalScrollBarDecorator extends WindowDecorator {
    public VerticalScrollBarDecorator (Window decoratedWindow) {
        super(decoratedWindow);
    }

    @Override
    public void draw() {
        super.draw();
        drawVerticalScrollBar();
    }

    private void drawVerticalScrollBar() {
        // draw the vertical scrollbar
    }

    @Override
    public String getDescription() {
        return super.getDescription() + ", including vertical scrollbars";
    }
}

They are overriding the draw method, but they are still calling the original method. This functionality should be included in some way. Without it, decorators and mixins will be severely gimped.

[jakobo]

The last I had heard on this discussion was that @dylnhdsn and @krisk were going to sync up on use cases. The uncovered problem is that decorators may not reside in the same scope as the instance, making method-wrapping very difficult. This is useful in decoration in order to have access to the original method if a decorator needs to replace functionality.

The working proposal is to pass the instance as a second parameter to decoration:

var MyDecorator = function(base, inst) {
  return {
    reallyCoolMethod: function() {
      // skips the current this.reallyCoolMethod on the instance, goes straight to
      // the parent method
      base.reallyCoolMethod.apply(this, arguments);

      // calls the original this.reallyCoolMethod, in case a wrapping behavior is desired
      inst.reallyCoolMethod.apply(this, arguments);
    }
  };
}

The alternate syntax is to rely on Fiber's this context inside of a decorator. During decoration, this refers to the object being decorated. The following is valid for Fiber, and would require no code change (but would likely involve a bit of documentation):

var MyDecorator = function(base) {
  var oldReallyCoolMethod = this.reallyCoolMethod;
  return {
    reallyCoolMethod: function() {
      // skips the current this.reallyCoolMethod on the instance, goes straight to
      // the parent method
      base.reallyCoolMethod.apply(this, arguments);

      // calls the original this.reallyCoolMethod, in case a wrapping behavior is desired
      oldReallyCoolMethod.apply(this, arguments);
    }
  };
}

Kiro and Dylan, please update the thread with what you think the best course of action is.

[dylnhdsn]

I just started using the second option. Like this…

var control = this, original = { someMethod: control.someMethod, someOtherMethod: control.someOtherMethod };

That first option might be nice for some syntactical sugar, but probably isn't really need. I think the downside there is that every single decorator will always copy the current instance. This sugar is easy to replicate when needed and would come at a cost to performance.

My main concern when bringing it up was that base didn't seem consistent. After Kiro explained it, it made more sense, but to me, it still has no apparent value. I just wanted to make sure I wasn't doing something wrong or that there was a bug with Fiber.

[jakobo]

Okay, let's just roll with the second option for now. Fiber's goal is to stay small and fast, so we can revisit this if there's enough demand in decoration for a more obvious pattern.