davej / angular-classy

Cleaner class-based controllers with Angular 1
http://davej.github.io/angular-classy/
813 stars 27 forks source link

Interesting Scoping Issues #16

Closed amcdnl closed 10 years ago

amcdnl commented 10 years ago

Okay, first off, I dont know why this type of thing is not in angular base...this is exactly what i've been looking for since i started working on angular! awesome work!

I tried to convert one of my most basic controllers code here. you can see the pre-classy integration and post-classy integration.

I'm not sure how to handle this but my this usage is crazy and the .bind(this) brings back backbone memories ;) . So is my implementation on 'par' and is there a better way to handle all the this references. I think it would be a lot better if i didn't have to use this on all the injected modules, what do you think?

davej commented 10 years ago

That's just the nature of doing class-based stuff with javascript I'm afraid. I use Coffeescript which uses the @ symbol as a shortcut to this, it also uses fat-arrow (=>) instead of skinny arrow (->) to denote if the function should be bound to the existing reference of this. So a lot of this stuff isn't much of an issue in Coffeescript.

That doesn't really help you if you're not willing to make the switch though. Here's a pattern that I would recommend that applies to javascript:

module.classy.controller({
    // ...
    saveApp: function() {
        this.AppsModel.create(model).then(this._afterSave);
    },
    _afterSave: function(xhr) {
        this.$scope.modal.hide();
        // Do more Stuff
    },
    // ...
};

By separating your code into smaller functions then you don't need to do .bind(this) because classy will automatically bind them for you. It's also just good practice in general.

davej commented 10 years ago

Just to summarise, here are the ways to deal with callbacks and other functions which change context:

1 - You can declare a reference to this, so that it can be used in callbacks:

var self = this;
self.$http.get('').success(function(data){ self.$.stuff = data; });

2 - You can use function.bind (or angular.bind if you need old IE support):

this.$http.get('').success(function(data){ this.$.stuff = data; }.bind(this));

3 - You can reference another Classy method and Classy will automatically bind correctly (this was fixed in the latest release so make sure you have 0.4.2):

app.classy.controller({
    // ...
    getTweets: function() {
        this.$http.get('').success(this._parseTweets);
    },
    _parseTweets: function(data) {
        this.$.stuff = data;
    }
});

4 - Or, if you use CoffeeScript (like me) then it's really easy and you can use the fat arrow syntax:

@$http.get('').success (data) => @$.stuff = data
amcdnl commented 10 years ago

@davej - Thanks for the quick reply! I totally agree with your points too.

Ya, I've used patterns like that in the past. I think it was just the injected modules that were 'odd feeling' on the this scope to me.

Eh, coffee script ... never been big into it but does provide some nice features for things like this.

amcdnl commented 10 years ago

@davej Have you done any Angular 1.3.0 beta testing yet?

davej commented 10 years ago

No, but it should just work, there's nothing in the changelog to indicate that there will be any issues. Let me know if you have any issues with 1.3.