Alex, I'll give this a try today and report back. Maybe we can have our cake and eat it too. :)
On Wed, Nov 7, 2018 at 8:29 AM Greg Dove <[email protected]> wrote: > > 'I'm not sure it is worth figuring out how far up the inheritance chain > the other private variable is and assigning numbers ' > > I was just meaning at simple class level - so not doing anything more than > that. The 'depth number' is the same for all private fields > Y extends Object > X extends Y > > in Y all privates are _$P1 suffix > in X all privates are _$P2 suffix > > nothing too sophisticated > > would that work? > > > On Wed, Nov 7, 2018 at 7:39 AM Alex Harui <[email protected]> > wrote: > >> 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 >> > > >> > > >> > > >> > >> > >> > >> >> >>
