Inline.
On 10/1/06, Scott J. Miles <sjmiles@xxxxxxxxxxxxx> wrote:
This "bug" has been posted many times. It's not a bug. You cannot call into
a superclass method that uses "inherited" from a method that was invoked by
"inherited". It does not work that way, never has, never was intended to.
"inherited" is just a slight enhancement of the "superclass" simplification.
If you think I was planning carefully to call superclass methods, you
are mistaken. It just happened after some revisions. And the behavior
was a complete surprise for me costing an hour of debugging. In the
complex code it is quite difficult to trace what method I call, who
calls my method, and who may override it. Referring to my toy example,
I call a regular method of the class, which turned out to be
overridden in the derived class at some point. More than that --- it
is very rare that my code does this particular chain of calls --- and
other branches work just fine. You can imaging my surprise when after
some innocent changes like piggy-backing on some method I got subtle
bugs.
Your fix works for your test case but breaks the actual intended
functionaliy of "inherited" which is to be able to call a series of
ancestral methods, e.g. "fillInTemplate".
This is precisely the reason I didn't go in the repository fixing the
"bug" cowboy-style. :-) I suspected that the code was this way for a
reason, and my change may break a lot of other stuff.
Dojo.declare is not designed to make JS into C++/Java/Other OOP language.
It's only a facility to build JS constructors with common boilerplate.
Believe it or not, it was not an exercise in OOP. It is actually AOP
--- do some stuff after or before actual work done in a base class
method. All I wanted is some event-based chaining. :-)
There are various solutions out there for the kind of functionality you are
looking for, but IMO they are too invasive. The "inherited" I included has
no impact on the normal usage of the objects.
I'll rewrite my code. The only problem I see is dojo.declare
introduces an unintuitive fragility, which requires a constant code
analysis to make sure that innocent changes will not break everything.
Basically calling a method, which can be overwritten, or calls in turn
a method like that, is sufficient to break everything.
Thanks,
Eugene
Regards,
Scott J. Miles
TurboAjax Group
http://www.turboajax.com
> -----Original Message-----
> From: dojo-escalate-bounces@xxxxxxxxxxxxxxx
> [mailto:dojo-escalate-bounces@xxxxxxxxxxxxxxx] On Behalf Of
> Eugene Lazutkin
> Sent: Sunday, October 01, 2006 9:21 AM
> To: dojo-escalate@xxxxxxxxxxxxxxx
> Subject: [Dojo-escalate] bug in dojo.lang declare
>
> Today I found a bug in otherwise excellent dojo.declare
> facility. I want this bug to be resolved quickly (it breaks
> dojo.gfx at the moment), and I don't feel it is a change I
> should do on my own => I escalate it.
>
> The bug manifests itself in following case:
>
> dojo.declare("X", Base, {
> a: function() {
> dojo.debug("X.a");
> },
> b: function() {
> dojo.debug("X.b");
> this.a();
> }
> });
>
> dojo.declare("Y", Base, {
> a: function() {
> dojo.debug("Y.a");
> this.inherited("a", []);
> },
> b: function() {
> dojo.debug("Y.b");
> this.inherited("b", []);
> }
> });
>
> var t = new Y();
> t.b();
>
> The last call has to print all 4 messages. In fact it prints only 3
> --- X.a is never called. It appears that the culprit is
> dojo.lang.declare.base._getPropContext() --- it returns
> either this.___proto or this, which breaks a search loop in
> dojo.lang.declare.base.inherited(): the latter case should be
> processed with do{}while() loop, but the former (with
> ___proto) should be processed with while(){} loop (we should
> search right away).
>
> My patch, which fixes the problem for me, is below:
>
> Index: declare.js
> ===================================================================
> --- declare.js (revision 5921)
> +++ declare.js (working copy)
> @@ -136,12 +136,18 @@
> },
> // searches backward thru prototype chain to find
> nearest ancestral instance of prop
> inherited: function(prop, args){
> - var p = this._getPropContext();
> + var p = this;
> do{
> -
> if((!p.constructor)||(!p.constructor.superclass)){return;}
> - p = p.constructor.superclass;
> + if(p.___proto){
> + p = p.___proto;
> + }else if(p.constructor &&
> p.constructor.superclass){
> + p = p.constructor.superclass;
> + }else{
> + return;
> + }
> }while(!(prop in p));
> return (dojo.lang.isFunction(p[prop]) ?
> this._inherited(p, prop,
> args) : p[prop]);
> +
> }
> }
>
> Basically I removed a call to _getPropContext, and slightly
> modified the search loop, including a test for ___proto in
> it. I cannot tell, if it can break something else, but it
> works for me.
>
> Please assess the problem ASAP --- it prevents me from
> checking in the latest refactored dojo.gfx code, and possibly
> affects some code out there.
>
> Thanks,
>
> Eugene
>
--
No virus found in this outgoing message.
Checked by AVG Free Edition.
Version: 7.1.407 / Virus Database: 268.12.10/459 - Release Date: 9/29/2006
_______________________________________________
Dojo-escalate mailing list
Dojo-escalate@xxxxxxxxxxxxxxx
http://dojotoolkit.org/mailman/listinfo/dojo-escalate
