After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 565029 - Style guide - change recommendation for inheritance
Style guide - change recommendation for inheritance
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks:
 
 
Reported: 2008-12-18 18:48 UTC by Owen Taylor
Modified: 2009-03-20 18:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Change recommendation for inheritance in the style guide (3.41 KB, patch)
2008-12-18 19:49 UTC, Owen Taylor
none Details | Review

Description Owen Taylor 2008-12-18 18:48:35 UTC
The style guide recommends:

[...]
Lang.copyProperties(Base.prototype, Sub.prototype);

Sub.prototype._init = function(foo, bar) {
[...]

The biggest problem I have with this style is the way methods are
declared when you add a base class, you switch from the 

 _init : function(...) {
 }

style to the less-indented style above, which requires reindentating
the whole file (also makes moving methods between files tedious.).
That could be solved by doing instead:

Lang.copyProperties({
   _init : function(foo, bar) {
   ...
}, Sub.prototype);

But as Johan Dahlin has pointed out, as long as we are using Spidermonkey
extensions, and easier and more efficient thing to do is to modify
the prototype chain:

Sub.prototype = {
    __proto__: Base.prototype,
    _init : function(foo, bar) {
    ...
};

(I think this will work fine with JavascriptCore from webkit as well,
though haven't tested.)

https://developer.mozilla.org/en/Core_JavaScript_1.5_Guide/Inheritance
has a good overall description of the area. It has a recommendation to
encapsulate assigning to __proto__ inside an extends() function, but
I don't really see a reason to do that when not coding for the web.

If changing the style guide sounds good, I can prepare a patch.
Comment 1 Havoc Pennington 2008-12-18 18:51:56 UTC
Yes, when doing the style guide I thought assigning to __proto__ wasn't allowed, but apparently it's just not portable, which we don't care about.

So, sounds good.

I agree that an extends() just obfuscates, it's clearer to just write it out.
Comment 2 Owen Taylor 2008-12-18 19:49:46 UTC
Created attachment 124952 [details] [review]
Change recommendation for inheritance in the style guide

Instead of Lang.copyProperties(), recommend the (non-portable) technique
assigning  __proto__ in the subclass to the base class's prototype.
Comment 3 Havoc Pennington 2008-12-18 20:02:28 UTC
"In portable JavaScript code you'll often see a different technique used to get this prototype chain" seems a little misleading though it's true, because there is a difference in outcome of the techniques (i.e. whether Sub.prototype is an Object or a Base).

"prototype chain" means the __proto__ chain not the Constructor.prototype, but maybe it's clearer to say "you'll often see a different technique used to set Sub.prototype.__proto__ to Base.prototype" and then "in this case Sub.prototype will be a Base, while with our recommended approach, Sub.prototype will be an Object"

Or something like that. Maybe adding text doesn't really help :-P Or maybe what's really needed is more background or a link to https://developer.mozilla.org/en/Core_JavaScript_1.5_Guide/Inheritance

Anyway, looks good.
Comment 4 Colin Walters 2009-03-20 18:46:15 UTC
Applied.