Problem of inheritance

[Matěj Cepl]

Hi,

I am sorry to bother you with my problems, but I am really in the end of
my wits on this.

I am trying to rewrite my Jetpack-prototype script
(https://fedorahosted.org/bugzilla-triage-scripts/browser/bugzillaBugTriage.js)
into the Brave New World of Jetpack-SDK and there is something very
wrong about the way how I am doing inheritance apparently (mostly all
code is actually taken over from the -prototype script and just split
into multiple modules).

The script is supposed to work both on any reasonable bugzilla instance,
but it has also some additional properties (and methods) specific for my
primary bugzilla, bugzilla.redhat.com. I guess, this is a good use case
for inheritance, right?

For generating daughter objects I use (almost verbatim) function heir
from David Flanagan "JavaScript: The Definitive Guide" as one method
provided by util module:

exports.heir = function heir(p) {
function f() {};
f.prototype = p.prototype;
return new f();
};

------------------------------------------------

Then there is a parent object (in a special module bzpage) with one
additional method:

var util = require("util");
var simpleStorage = require("simple-storage");

var BZPage = exports.BZPage = function BZPage(doc, config) {
// initialize dynamic properties
this.doc = doc;
console.log("Now we are inside!");
};

BZPage.prototype.getURL = function getURL () {
console.log("url = " + this.doc.location.href);
return this.doc.location.href;
};

------------------------------------------------

Daughter object is in another module:

var util = require("util");
var BZPage = require("bzpage").BZPage;

var RHBugzillaPage = exports.RHBugzillaPage =
function RHBugzillaPage(doc, config) {
BZPage.call(this, doc, config);

console.log("location = " + this.doc.location);

//this.bugId = this.getBugId();
this.bugId = util.getBugNo(this.doc.location.href);
console.log("doc = " + this.doc.location);
console.log("bug number = " + this.bugId);
console.log("this = " + this);
console.log("prototype = " + this.prototype);
console.log("constructor = " + this.constructor);
console.log("this.getURL = " + this.getURL);
//this.bugId = this.getBugId();
console.log("bug URL = " + this.getURL());
console.log("Now we are outside!");
}; // END OF RHBugzillaPage CONSTRUCTOR

RHBugzillaPage.prototype = util.heir(BZPage.prototype);
RHBugzillaPage.prototype.constructor = RHBugzillaPage;

------------------------------------------------

Whole this machinery is activated by this main module:

var util = require("util");
var file = require("file");
var myStorage = require("simple-storage").storage;
var browser = require("tab-browser");
var JSONURL = "http://matej.ceplovi.cz/progs/data/RH_Data-packages.json";

var config = {};

var matches = [
"https://bugzilla.redhat.com/show_bug.cgi.*",
"https://bugzilla.mozilla.org/show_bug.cgi.*"
];

function initialize(callback) {
util.loadJSON(JSONURL, function(parsedData) {
config = {};
callback(config);
}, this);
}

function isOurPage(window, matchingURLs) {
if ("window" in window) {
window = window.window;
}
var url = window.location.href;

return matchingURLs.some(function (element,i,a) {
return new RegExp(element).test(url);
});
}

exports.main = function main(options, callbacks) {
initialize(function (config) {
browser.whenContentLoaded(
function(window) {
var doc = window.document;
if (isOurPage(window, matches)) {
var curPage = new
require("rhbzpage").RHBugzillaPage(doc, config);
} else {
console.log("Not our page: " + window.location.href);
}
});
});
};

------------------------------------------------

Now this whole seems to me like it should work, but when I do

cfx run -a firefox

and go to https://bugzilla.redhat.com/show_bug.cgi?id=998 the script
gets correctly activated, objects seems to be created as well, but
getURL function property just isn't there:

(jetpack-sdk)johanka:bugzilla-triage$ cfx run -a firefox
++ date +%j
+ odd=1
+ '[' 1 -eq 1 ']'
+ setarch i686 /usr/bin/firefox -profile /tmp/tmpVNjQNe.mozrunner
info: Now we are inside!
info: location = https://bugzilla.redhat.com/show_bug.cgi?id=998
info: doc = https://bugzilla.redhat.com/show_bug.cgi?id=998
info: bug number = 998
info: this = [object Object]
info: prototype = undefined
info: constructor = function Object() {
[native code]
}
info: this.getURL = undefined
error: An exception occurred.
Traceback (most recent call last):
File
"resource://jid0-uxmbewgoltuuuqrhkhrr7hw3iqy-jetpack-core-lib/errors.js", line
49, in
return callback.apply(this, arguments);
File
"resource://jid0-uxmbewgoltuuuqrhkhrr7hw3iqy-jetpack-core-lib/tab-browser.js",
line 265, in eventHandler
callback(event.target.defaultView);
File
"resource://jid0-uxmbewgoltuuuqrhkhrr7hw3iqy-bugzilla-triage-lib/main.js",
line 53, in
var curPage = new require("rhbzpage").RHBugzillaPage(doc, config);
File
"resource://jid0-uxmbewgoltuuuqrhkhrr7hw3iqy-bugzilla-triage-lib/rhbzpage.js",
line 27, in RHBugzillaPage
console.log("bug URL = " + this.getURL());
TypeError: this.getURL is not a function

How come that this.doc property seems to be created and inherited
correctly, but this.getURL just doesn't make it? And why is
this.prototype undefined?

I am beating my head against this problem for almost a day, read
Flanagan on objects like the Holy Writ again, and I still don't see
anything wrong. It is probably some really stupid newbie bug, but I
really don't see it.

Would anybody here be so kind and help me here, please? (for interested,
whole code is in a Mercurial repository
http://www.bitbucket.org/mcepl/bugzilla-test, branch OOPtesting branch).

Thank you for any response.

Matěj

--
Our lives are spectacles of powerlessness.
-- Richard Rohr

[Felipe Gomes]

Hey Matej,

yesterday I took a look on IRC but now that I see the usage I think I guess what the problem is. It's not an inheritance problem,  it's the difficult precendence of the _new_ operator.  The following line:

 var curPage = new require("rhbzpage").RHBugzillaPage(doc, config);

doesn't do what you intended. The new operator applies to the require function, not your constructor, so you have to add some parentheses around it, or for clarity do it on two steps.

Also, I suggest you export your constructors using the api-utils.publicConstructor method. It creates a constructor that doesn't require that you use the new operator, which would have made your line above work simply by removing the |new|.


I've also been bitten by this problem, and it seems to cause subtle errors that are hard to debug. I was wondering if we should modify the require function to detect when the new operator was used on it and throw an error. This would make it dead easy to find problems like this. Any thoughts? I can write a patch for that if people like the idea.


- Felipe

2010/6/12 Matěj Cepl <mc...@redhat.com>

[Marcio Galli]

So you can patch these sort of optimizations related to patters of coding in the SDK code too? This is very cool Felipe. If you do it ping the patch example update.

m

--
You received this message because you are subscribed to the Google Groups "mozilla-labs-jetpack" group.
To post to this group, send email to mozilla-la...@googlegroups.com.
To unsubscribe from this group, send email to mozilla-labs-jet...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/mozilla-labs-jetpack?hl=en.

--
blog.taboca.com

[Matěj Cepl]

Dne 12.6.2010 20:16, Felipe Gomes napsal(a):

> var curPage = new require("rhbzpage").RHBugzillaPage(doc, config);
>
> doesn't do what you intended. The new operator applies to the require
> function, not your constructor, so you have to add some parentheses
> around it, or for clarity do it on two steps.

Hmm, so plain

var construct = require("rhbzpage").RHBugzillaPage;
if (isOurPage(window, matches)) {
var curPage = new construct(doc, config);

} else {
console.log("Not our page: " + window.location.href);
}

didn't help. Still the same error (this.getURL is not a function).

Neither

var curPage = new (require("rhbzpage").RHBugzillaPage)(doc, config);

did.

> Also, I suggest you export your constructors using the
> api-utils.publicConstructor method. It creates a constructor that
> doesn't require that you use the new operator, which would have made
> your line above work simply by removing the |new|.

Just using this (i.e., using publicConstructor in bzpage and rhbzpage
modules and removing new from the line) makes things actually even worse
... now even this.doc is not known.

The same result (no this.doc) happens even when I combine the two (using
publicConstructor in *page modules AND var construct).

http://www.bitbucket.org/mcepl/bugzilla-test repo has been updated to
this state.

> I've also been bitten by this problem, and it seems to cause subtle
> errors that are hard to debug. I was wondering if we should modify the
> require function to detect when the new operator was used on it and
> throw an error. This would make it dead easy to find problems like this.
> Any thoughts? I can write a patch for that if people like the idea.

I guess this deserves a bug in b.m.o, right?

Best,

Matěj

--
According to the Franciscan priest Richard Rohr, spirituality is
not for people who are trying to avoid hell; it is for people
who have been through hell. In many ways, spirituality is about
what we do with our pain. And the truth is, if we don't
transform it, we will transmit it.
-- Al Gustafson

[Atul Varma]

On 6/12/10 11:16 AM, Felipe Gomes wrote:
> I've also been bitten by this problem, and it seems to cause subtle
> errors that are hard to debug. I was wondering if we should modify the
> require function to detect when the new operator was used on it and
> throw an error. This would make it dead easy to find problems like
> this. Any thoughts? I can write a patch for that if people like the idea.

That is a genius idea!! If you write a patch, not only will I be really
grateful, I will also buy you a beer or other non-alcoholic beverage, as
I have been bitten by this too and I'm sure it's going to happen to more
people.

- Atul

[Myk Melez]

On 06/12/2010 02:27 PM, Atul Varma wrote:
> On 6/12/10 11:16 AM, Felipe Gomes wrote:
>> I've also been bitten by this problem, and it seems to cause subtle
>> errors that are hard to debug. I was wondering if we should modify
>> the require function to detect when the new operator was used on it
>> and throw an error. This would make it dead easy to find problems
>> like this. Any thoughts? I can write a patch for that if people like
>> the idea.
> That is a genius idea!!

Bonus points if you can figure out a way to to make "require" a catchall
that transparently proxies to the actual constructor when it is invoked
as a constructor!

-myk

[mcepl]

On Jun 12, 11:26 pm, Matěj Cepl <mc...@redhat.com> wrote:
> I guess this deserves a bug in b.m.o, right?

In the end, I did that (https://bugzilla.mozilla.org/show_bug.cgi?
id=572566) just to learn that I am an idiot. Case closed.

Thanks