> On 2012-02-13 23:12:07, Henry Saputra wrote:
> > trunk/features/src/main/javascript/features/actions/actions_container.js,
> > line 291
> > <https://reviews.apache.org/r/3873/diff/2/?file=74485#file74485line291>
> >
> > Nothing in particular, just making it easier to read for me while
> > fixing the array errors.
I ask because the code you introduced is often larger (even when closure
complied), with the exception of perhaps the var statements.
When traversing an array with an index, doing
for(var i=0,b;b=a[i];i++){}
is smaller than:
for(var i=0;i<a.length;i++){var b=a[i];}
by about 13 characters
Yeah sure, just 13 characters :) but that's for each for loop. This is
something that closure doesn't optimize for you because of the trickiness of
the truthy assignment. You as a programmer can know if the value assigned
might possibly be falsy, and should not use this technique in this case (so
closure can't really know that, unless there is an annotation I'm unaware of)
like if a value in an array might legitimately be null, 0, undefined or ''.
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3873/#review5062
-----------------------------------------------------------
On 2012-02-12 01:46:58, Henry Saputra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3873/
> -----------------------------------------------------------
>
> (Updated 2012-02-12 01:46:58)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> Some of the iterations in the actions_container.js file are using "in"
> operator without calling hasOwnProperty to make sure its not going up to
> prototype chain (http://yuiblog.com/blog/2006/09/26/for-in-intrigue/). It
> causes trouble when executing in some browsers like FF5.
>
> Small cleanup for actual array to just simply used for-loop with index.
>
>
> Diffs
> -----
>
> trunk/features/src/main/javascript/features/actions/actions_container.js
> 1243090
>
> Diff: https://reviews.apache.org/r/3873/diff
>
>
> Testing
> -------
>
> Pass JS unit tests and manual test with common container and action container.
>
>
> Thanks,
>
> Henry
>
>
