Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helper for a mixin to override parent's method. #11

Closed
MatejJan opened this issue Apr 3, 2015 · 3 comments
Closed

Helper for a mixin to override parent's method. #11

MatejJan opened this issue Apr 3, 2015 · 3 comments

Comments

@MatejJan
Copy link
Member

MatejJan commented Apr 3, 2015

This is a potential function for BlazeMixin (the BlazeComponent extension that might or might not be part of the package).

I have a hard time coming to terms that the implementer of the component needs to know what functions the mixins are overriding (or the component is overriding from them, depending which way you look at it).

We'll see in practice how often this would be useful, but if more simple cases like PersistentInputMixin (previously Frozen) from the demo could function by simply augmenting the parent class, a helper for that might be worth it.

So you would say:

override: ->
  value: (_super) ->
    @storedValue.get() or _super()

This is exactly how we have written this function through inheritance:

value: ->
  @storedValue.get() or super

(We could also go for Super instead of _super.)

I did not test what happens if two mixins override the same function … I guess they would just get chained in order of mixins (assuming they are instantiated in that order), so it's like inheritance from the component on top and mixins going from first to last. So the last mixin is also at the bottom of this 'inheritance' chain. It's up to them to call _super() to travel up the chain to the component on top. Which is not the nicest, since the idea of composition is that mixins shouldn't have to worry about each other.

Anyway, the code to make override work is:

  onCreated: ->
    super
    parent = @mixinParent()
    for name, func of @override()
      old = parent[name]
      parent[name] = (args...) =>
        _super = (args...) ->
          old.call parent, args...
        func.call @, _super, args...

I wonder if I could modify the code so that all mixins who overrided a function are called in parallel instead of as a chain. And they all get the same _super that is component's implementation. A downside of this is that _super can get called multiple times, which doesn't play well with functions that have side effects. Neither is it clear what to actually return as the final value from the function (first result? last?).

@mitar
Copy link
Member

mitar commented Apr 5, 2015

So yea, this can be a big discussion. How to provide in some way multiple inheritance. The approach we are currently taking is that component has to provide an explicit extension point by calling one of call* methods for mixins. Another would be that mixins can influence the component on their own.

In some way, in OOP by creating a method you create such an extension point. Which then child implementations can extend. If you do not put a piece of logic into a method then children are unable to extend that piece of logic.

Currently we in some way require definition of an extension point twice: by creating a method and by calling call*. What you are arguing for in some way is that it should be enough just to create a method. And then if a mixin wants to extend that method, we should offer some API to do that.

In some way parent methods do not call callChildren here. They just call upwards. And mixins could do the same. Just calling side-wise (if imagine inheritance to be top to bottom, and mixins left to right).

@mitar
Copy link
Member

mitar commented Apr 6, 2015

One question to consider is what if component does not provide this method. Probably then mixin should just set it. But then maybe we do not need anymore that template helpers are searched in mixins and components, but just components, and if mixin wants to add a template helper, it overrides (or sets) it on the component.

Same could be done also for event handlers.

@mitar
Copy link
Member

mitar commented Apr 6, 2015

After more thought I think the main confusion is coming from the fact that mixins in current design in some way come after a component. So component is called first. So if component C adds mixins A, B, and C, then first component C will be called, then A, then B and so on. A cannot override things from C. But C and override things from A (or decide to call it, or ignore it, or whatever).

Same goes for dependencies mixin A introduces, A1 and A2, for example. They are added after A. So the order becomes C, A, A1, A2, B and so on. So A can then override things from A1 and A2.

I think it is a good design that we allow overriding or extending only in one direction. In this way it works the same as your _super, just in the other direction. Because I think current API is more powerful (you can decide how exactly you want to continue, instead of having just _super, you might want to continue by calling other methods, not just your own method (_super gives you access just to overridden methods, not to the whole "super" object)) I am opting for keeping the current design, but make it more consistent that the order is that components are first, and then mixins, with mixins going in the order they are listed.

If you want some functionality to be overridden by a mixin, you have to move it to a mixin which comes after your mixin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants