Hi Greg,

#1 is a good idea and will help, but the value for that variable is still 
pushed far to the right in the debugger and pushed other code far to the right 
in the source window.

#2 and similar ideas will help.  I'm not sure it is worth figuring out how far 
up the inheritance chain the other private variable is and assigning numbers.  
I considered using some sort of hash of the fully qualified classname so each 
prefix/suffix would be, say 8 characters long.

That said, though, in the framework, I think we should not have private name 
collisions.  I found some duplicate code that would be caught if we turned on 
errors for these collisions, and would encourage us to use overriding which 
enables other folks to do overrides.

So, if you want to muck around in this code, feel free to switch the order in 
#1, but I think the next piece of work is to turn on errors for collisions in 
the royale-asjs build.

My 2 cents,
-Alex

On 11/6/18, 10:07 AM, "Greg Dove" <[email protected]> wrote:

    Hi Alex,
    
    "IMO, it makes debugging the framework really painful."
    
    I feel your pain. I didn't try switching it off yet, but I wonder if it can
    be on by default with a different approach:
    example:
    
    this.org_apache_royale_jewel_beads_models_DropDownListModel__selectedIndex
    
    1. have the 'private' part of the name as a suffix instead of prefix. It
    means you can read the important part easier (doesn't help with the very
    long lines with multiple references on them, but might be easier in general)
    
    2. maybe the class name approach is not needed?
    the problem domain is more about conflicts in inheritance chain rather than
    class naming, is it not?
    so if
    -local private name is __selectedIndex
    -and the current class is 5 levels away from Object base class
    would it not be possible to use t
    this.__selectedIndex_$P5
    or similar?
    
    if this 2nd part makes sense (still pondering it) then it could probably be
    on by default.
    
    
    
    On Thu, Nov 1, 2018 at 5:21 AM Alex Harui <[email protected]> wrote:
    
    > Hi Kenny,
    >
    > I went and made the changes to decorate private variable names.  I made it
    > an option that's on by default.  IMO, it makes debugging the framework
    > really painful.  I'm probably going to turn the flag off in the framework
    > and have a policy that we don't use the same private variable names.  It
    > shouldn't affect what users do.
    >
    > Also, turning the option off revealed a fair amount of duplicated code.
    > Sometimes we copy a file to form the basis of a subclass and then don't
    > clean up everything and you don't find out about private methods if they
    > have their names decorated.
    >
    > -Alex
    >
    > On 10/28/18, 5:22 AM, "Kenny Lerma" <[email protected]> wrote:
    >
    >     Alex, this is absolutely needed.  The same private variables names in
    > base
    >     class and subclass are quite common.  Since this only affects debug,
    > there
    >     is no harm in the decorator and maintains consistency with flash.
    >
    >
    >     On Sat, Oct 27, 2018, 9:05 PM Alex Harui <[email protected]>
    > wrote:
    >
    >     > Hi,
    >     >
    >     > It appears that in Flash, private variables are actually in a custom
    >     > namespace.  This means you can have private APIs in a base class and
    >     > private APIs in a subclass with the same name (and aren't overrides)
    > and
    >     > everything "works".  IOW:
    >     >
    >     > package org.apache.royale.core {
    >     > public class BaseClass {
    >     >    private var foo:String = "foo,";
    >     >    public function BaseClass() {
    >     >       trace(foo);
    >     >    }
    >     > }}}
    >     >
    >     > package org.apache.royale.core {
    >     > public class MyClass {
    >     >    private var foo:String = "bar!";
    >     >    public function MyClass() {
    >     >       super();
    >     >       trace(foo);
    >     >    }
    >     > }}}
    >     >
    >     > var baz:MyClass = new MyClass();  // outputs foo,bar!
    >     >
    >     > This is true for private functions and getters/setters as well.
    > However,
    >     > this currently does not work in JS, since the transpiled code looks
    > like:
    >     >
    >     > org.apache.royale.core.BaseClass = function() { trace(this.foo) }
    >     > org.apache.royale.core BaseClass.prototype.foo = "foo,";
    >     >
    >     > And
    >     >
    >     > org.apache.royale.core MyClass = function() { trace(this.foo} }
    >     > org.apache.royale.core MyClass.prototype.foo = "bar!";
    >     >
    >     > So you will get "bar!bar!";
    >     >
    >     >
    >     > The MX Charts code uses the same private API names in base classes
    > and
    >     > subclasses.  I don't know why they didn't use protected methods and
    >     > override them, so I'm going to change the Charts code to use
    > overrides just
    >     > to keep making progress.
    >     >
    >     > I'm wondering if anybody else uses the same private API name in base
    >     > classes and subclasses.  The theoretical fix is to have the compiler
    >     > generate a decorated name.  That's what the compiler already does 
for
    >     > mx_internal APIs and other custom namespace APIs, but I think it
    > would make
    >     > our code fatter and uglier to decorate private API names, so I'm
    > tempted to
    >     > have the compiler emit an error or warning instead.
    >     >
    >     > In order to guarantee uniqueness, we'd have to decorate with the
    > fully
    >     > qualified name of the class.  Then the transpiled code would look
    > like:
    >     >
    >     > org.apache.royale.core.BaseClass = function() {
    >     > trace(this.org_apache_royale_core_BaseClass_foo) }
    >     > org.apache.royale.core
    >     > BaseClass.prototype.org_apache_royale_core_BaseClass_foo = "foo,";
    >     >
    >     > And
    >     >
    >     > org.apache.royale.core MyClass = function() {
    >     > trace(this.org_apache_royale_core_MyClass_foo} }
    >     > org.apache.royale.core
    >     > MyClass.prototype.org_apache_royale_core_MyClass_foo = "bar!";
    >     >
    >     > IMO, that's a painful change to the transpiler, so I want to make
    > sure we
    >     > really need to do this.  I think it won't impact minified size, but
    > it will
    >     > be noticeable when debugging the JS.
    >     >
    >     > Thoughts?
    >     > -Alex
    >     >
    >     >
    >     >
    >
    >
    >
    

Reply via email to