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
>
>

Reply via email to