Skip to content

Refactor all code base on #1#2

Open
mytharcher wants to merge 4 commits intodfkaye:masterfrom
mytharcher:master
Open

Refactor all code base on #1#2
mytharcher wants to merge 4 commits intodfkaye:masterfrom
mytharcher:master

Conversation

@mytharcher
Copy link
Copy Markdown

  • Remove new operator for new class definition. Change to directly function invoking.
  • Combine single class definition and extend definition.
  • Remove __super__ keyword. Because it may confuse user when using this.__super__.xxx to invoke method from 2 or more layer upper.
  • Change static inheritance by default but not override.
  • Update README.md and do some typo fix.

@dfkaye
Copy link
Copy Markdown
Owner

dfkaye commented Nov 18, 2014

This is a big change set. Thank you for going through all that ~ and making sure the tests pass :).

The only change I'm really not comfortable with is item 3, "Remove __super__ keyword. Because it may confuse user when using this.__super__.xxx to invoke method from 2 or more layer upper."

I used __super__ on each instance so that I would not have to refer to a base constructor by its name in any of the prototype methods, as in

Manager = Constructor(managerDef, Employee);

where

managerDef.constructor = function () {
  Employee.call(this, args...);
}

or

managerDef.method = function () {
  Employee.method.apply(this, args...);
}

By using

managerDef.constructor = function () {
  this.super(args...);
}

or

managerDef.method = function () {
  this.super.method(args...);
}

I don't have to pass this, I don't have to specify call or apply, and I can swap out the base class at the Constructor call without having to change all the base class references everywhere else.

Would it be possible to update the pull request so as to support the __super__ syntax?

@mytharcher
Copy link
Copy Markdown
Author

Thanks for reviewing.

I can understand your requirement about super. In fact, I also like a super if it would not confuse user. But this problem in JavaScript is a nature of language and can't be resolved. Just think about a 3 layers inheritance:

var Animal = Constructor({
    constructor: function(name) {
        this.name = name;
    },
    shout: function (str) {
        alert(this.name + str);
    }
});

var Dog = Constructor({
    constructor: function(name) {
        this.__super__(name); // Dog.prototype.__super__ = Animal
    },
    shout: function () {
        alert('wow~');
    }
}, Animal);

var Husky = Constructor({
    constructor: function (name) {
        this.__super__(name); // Husky.prototype.__super__ = Dog
        // What will happen by calling super here?
        // 1. call `Dog.call(this, name)`
        // 2. call `this.__super__(name)` in Dog constuctor, but `this` point to Husky instance
        // 3. call `Dog.call(this, name)` again, because the `this` never changed
        // ... dying circle
    }
}, Dog);

If we want to fix this, we need to write a compiling function automatically compute how many layers should find up to the proper ancestor. I did this a few years before, but finally it was abandoned. Because it is not a good way by compiling all method, or even only compiling the constructor. And I support the constructor should not be inherited by default as in Java.

We can make a compromise on this which avoid parent class name to be called in subclass by using SubClassName.__super__:

var Husky = Constructor({
    constructor: function (name) {
        Husky.__super__.call(this, name)
    },
    shout: function () {
        Husky.__super__.prototype.shout.call(this);
    }
}, Dog);

But we can't go further.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants