cocos2d / cocos2d-js

cocos2d-x for JS
http://www.cocos2d-x.org
MIT License
1.86k stars 490 forks source link

var self = this; #1329

Open adamlwgriffiths opened 9 years ago

adamlwgriffiths commented 9 years ago

I couldn't not raise this.

157         var self = this;

277         var self = this, ...,

299         var self = this, ...;

324         var self = this, ...;

These are scattered throughout this file and probably many other places.

http://thedailywtf.com/articles/_0x23_include__0x22_pascal_0x2e_h_0x22_

http://www.cocos2d-x.org/reference/html5-js/V3.2/symbols/src/_Users_panda_StudyWork_Cocos_cocos2d-js_frameworks_cocos2d-html5_cocos2d_core_sprites_CCSpriteFrameCache.js.html

lassade commented 9 years ago

This may look silly but its a "good" pratice. Javascript changes "this" varaible reference according with the owner of the function, so using self assure the value of the varaible points to the object that you whant.

// run this a = function () {} b = function () {} b.prototype.f = function () { var A = new a(); A.f = function () { return this }; return A; } var A = (new b()).f (); A.f(); // returns a

// then this a = function () {} b = function () {} b.prototype.f = function () { var self = this; var A = new a(); A.f = function () { return self }; return A; } var A = (new b()).f (); A.f(); // returns b

adamlwgriffiths commented 9 years ago

You're defending the use of a dirty bomb to dig a hole. There are times and places where obscure 'methods' are required. That is not the situation here. There are no callbacks. No passed functions. No closures.

The usage of this and self are even interchanged.

152         //Is it a SpriteFrame plist?
153         var dict = this._frameConfigCache[url] || cc.loader.getRes(url);
154         if(!dict || !dict["frames"])
155             return;
156 
157         var self = this;
158         var frameConfig = self._frameConfigCache[url] || self._getFrameConfig(url);

This code is simply, bad.

lassade commented 9 years ago

Yeah i see that, maybe you can send a PR with a reviewed code style. They use the google javascript style guide https://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml.

pandamicro commented 9 years ago

Hi, @adamlwgriffiths , Thanks for noticing

We changed this to self for the purpose of increasing compress rate in Closure Compiler. We thought self will be compressed to something like a which is shorter than this then be used through the whole method.

But it turns out Closure Compiler will simply replace them with this again. So you are right, we shouldn't do the replacement. We will change it back.