Alex, I agree, it seems whatever prompted this was elsewhere, but the outcome is IMO (a small amount of) better framework code in CSSUtils. I would take this as a small win - nothing is broken, and a utility method is theoretically slightly faster.
On Wed, Mar 15, 2017 at 5:19 PM, Alex Harui <aha...@adobe.com> wrote: > Thanks for running the test. Maybe I'm not understanding the issue, but > here's my summary. > > Justin was getting a compile error where code that was known to work > wouldn't compile because there was only one argument passed to parseInt in > ActionScript source. > > ActionScript defines parseInt as having one required parameter and one > default parameter so it should have compiled. > > Thus, the compile error was likely due to the bad typedefs build Justin > referred to earlier in a separate thread. > > It would not be my recommendation to have us add default parameters to all > of the places we could for "code clarity" or performance. Folks who write > code in ActionScript should know or can find from the documentation that, > for example, the second parameter to parseInt is optional and thus would > wonder why someone bothered to add it. If the second parameter isn't > there, the assumption should be that the default parameter is used. > > Now, if there is a performance advantage to having the output JS always > set 10 if the second parameter is not specified to parseInt, then that > sounds like a good idea. Please file a JIRA so we don't forget. > > But, IMO, we are writing ActionScript and we should not make a practice of > supplying default parameters. Please figure out why your typedefs aren't > building and remove the optional parameter for parseInt from CSSUtils.as. > > Thanks, > -Alex > > > On 3/14/17, 8:24 PM, "Greg Dove" <greg.d...@gmail.com> wrote: > > >I think code clarity is one thing, but performance is another - that > >should > >be faster, so I ran a quick check. > > > >I know it can vary across browsers, but > > > >var timeOne = function(){var d=new Date();var b=0; for (var > >i=0;i<10000000;i++) {b= parseInt(""+(127/255)*1000, 10) / 1000;} > >console.log(new Date().getTime()-d.getTime());} > >timeOne() > >approx 715 ms in my chrome over multiple runs > > > >var timeTwo = function(){var d=new Date();var b=0; for (var > >i=0;i<10000000;i++) {b= parseInt(""+(127/255)*1000) / 1000;} > >console.log(new Date().getTime()-d.getTime());} > >timeTwo () > >approx 870 ms in my chrome over multiple runs > > > >so (within the limits of this *very* basic test) I say keep it, for > >clarity > >and speed (about 20% faster) > > > >On Wed, Mar 15, 2017 at 3:26 PM, Justin Mclean <jus...@classsoftware.com> > >wrote: > > > >> Hi, > >> > >> > Please revert CSSUtils and investigate why parseInt is requiring the > >> second argument. > >> > >> Even if it is a typedef bug IMO passing the base there makes the code > >> intent clearer as the code is dealing with both base 16 and base 10 > >>numbers. > >> > >> This is the code in question: > >> public static function attributeFromColor(value:uint):String > >> { > >> var hexVal:String = value.toString(16); > >> if(value > 16777215) > >> { > >> //rgba -- return rgba notation > >> var rgba:Array = hexVal.match(/.{2}/g); > >> for(var i:int = 0; i < 4; i++) > >> { > >> rgba[i] = parseInt(rgba[i], 16); > >> } > >> rgba[3] = parseInt(""+(rgba[3]/255)*1000, 10) / 1000; > >> return "rgba(" + rgba.join(",") + ")"; > >> } > >> return "#" + StringPadder.pad(hexVal,"0",6); > >> } > >> > >> I added the “,10” to the second parseInt. > >> > >> What do others think? Should it stay or should it go? > >> > >> Thanks, > >> Justin > >