ded / klass

a utility for creating expressive classes in JavaScript
753 stars 78 forks source link

Unexpected .extend() behaviour when using "little curlies" method #5

Closed stephenmelrose closed 13 years ago

stephenmelrose commented 13 years ago

Hi,

I currently use Object.subclass() to write "class" based JS. I was recommended klass today and decided to check it out. I like it, especially the cleaner API, and the fact it's Node compliant.

I have a small issue though. Take the following,

var Object1 = klass({
    initialize: function() {
        console.log('Init 1');
    }
});

var Object2 = Object1.extend({
    initialize: function() {
        console.log('Init 2');
    }
});

var Object3 = Object2.extend({
    initialize: function() {
        console.log('Init 3');
    }
});

var myObject = new Object3();

The following is outputted,

Init 3
Init 3
Init 3

There are a few problems with this,

  1. I noticed your docs state "super class is automagically called" when extending and overriding the constructor. I think this should be more consistent with a non-constructor method where the super is only called if you explicitly state it should be using this.supr(), as with any other language OOP model.
  2. Regardless of point 1, I would expect each initialize() to execute in order, not the last most defined being excuted N times, where N is the number of extentions.

I would personally prefer point 1 to be implemented. Without it, I can't move to klass, and it just seems to make sense if you're implementing a Class model.

Thanks.

ded commented 13 years ago

I think @fat broke something. In the meantime, this will work as advertised.

var Object1 = klass(function() {
  console.log('Init 1');
});

var Object2 = Object1.extend(function() {
  console.log('Init 2');
});

var Object3 = Object2.extend(function() {
  console.log('Init 3');
});

var myObject = new Object3();

outputs:

Init 1
Init 2
Init 3
ded commented 13 years ago

another aside: this works too, which you might like:

var Object1 = klass({
    init: function() {
        console.log('Init 3');
        return this;
    }
});

var Object2 = Object1.extend({
    init: function() {
        console.log('Init 2');
        this.supr();
        return this;
    }
});

var Object3 = Object2.extend({
    init: function() {
        console.log('Init 1');
        this.supr();
        return this;
    }
});

var myObject = new Object3().init();
fat commented 13 years ago

This is fixed with the latest klass -- we implemented your "point 1" ... which is, when initialize is used, the constructor won't bubble.

thnaks for bringing this up!

stephenmelrose commented 13 years ago

Cheers boys.

To get around my point 2 we'll probably use,

var BaseKlass = new klass({
    initialize: function() {
        this.init();
    },
    init: function() {}
});

And use BaseKlass as our base object and init() as our constructor.

Thanks again.

ded commented 13 years ago

just remember that the auto-propagation (a feature i really like) is always there for you when you need it. simply pass a function as an argument to klass and all is magic.

fat commented 13 years ago

Hm... you shouldn't have to do that -- would you mind explaining again what's happening to you?

ded commented 13 years ago

i think he just likes it stylistically and has nothing to do with klass.

fat commented 13 years ago

With the new code, initialize will only be executed 1 time invocation ( you have to call supr directly).

fat commented 13 years ago

his new code won't work. well... it will.. but it's wrong to do that... initliaze does what he wants by default now.

ded commented 13 years ago

his new code will work. i think you're reading it wrong. he just wants to separate his init into a separate method. initalize will be called, then it calls init.

fat commented 13 years ago

in his initial example:

var Object1 = klass({
    initialize: function() {
        console.log('Init 1');
    }
});

var Object2 = Object1.extend({
    initialize: function() {
        console.log('Init 2');
    }
});

var Object3 = Object2.extend({
    initialize: function() {
        console.log('Init 3');
    }
});

The following is now outputted,

init 3
stephenmelrose commented 13 years ago

Balls. Sorry, I replied to this on my iPhone in the pub, got my point 1 and point 2 mixed up.

This works exactly as I would expect it to now. I thought you'd only fixed the weird executing the third initialize() three times bug, and not stopped it auto-executing too.

So to achieve,

Init 1
Init 2
Init 3

I can now write,

var Object1 = klass({
    initialize: function() {
        console.log('Init 1');
    }
});

var Object2 = Object1.extend({
    initialize: function() {
        this.supr();
        console.log('Init 2');
    }
});

var Object3 = Object2.extend({
    initialize: function() {
        this.supr();
        console.log('Init 3');
    }
});

var myObject = new Object3();

Which is exactly what I'd expect.

The code I posted was incase initialize() still automatically called the supr() for you. In my code the constructor would only exist on a base object and call my init() once, which I can then extend and call supr() within myself, essentially moving the constructor to another method giving me full control. But you sorted that so my point and code is completely a moo point.

So, cheers again boys and sorry for the confusion!

stephenmelrose commented 13 years ago

And as you pointed out @ded, you can still make it automatically call the supr() by using the non-"little curlies" method too. So now it's purely a stylistic choice. Lurvely!

fat commented 13 years ago

great -- glad everything is working for you :D