Google closure compiler warns on line in createVirtualEvent #5246

Closed
btrscott opened this Issue Nov 3, 2012 · 1 comment

Comments

Projects
None yet
2 participants

btrscott commented Nov 3, 2012

Google doesn't seem to line

    for ( i = props.length, prop; i; ) {   <<<<<<<<<<<<
        prop = props[ --i ];
        event[ prop ] = oe[ prop ];
    }

WARNING - Suspicious code. This code lacks side-effects. Is there a bug?

for ( i = props.length, prop; i; ) {
.....................................^

It's an unusual for structure from my view. Not saying it doesn't work but it sure looks non-standard.

Missing 3rd position incrementing function; seems to be handled within the look with the --i. Odd multiple assignment in first for position.

My guess would be:

    for ( i = props.length; i; ) {   <<<<<<<<<<<<
        prop = props[ --i ];
        event[ prop ] = oe[ prop ];
    }

But even that is not straight forward. Maybe:

    for ( i = props.length-1; i<0; i--) {   <<<<<<<<<<<<
        prop = props[ i ];
        event[ prop ] = oe[ prop ];
    }

Course counting up is more traditional but I don't know if there is another reason requiring that you go backwards.

    for ( i = 0; i<props.length; i++) {   <<<<<<<<<<<<
        prop = props[ i ];
        event[ prop ] = oe[ prop ];
    }

Maybe the original code is highly optimized to those who understand the lower workings of Javascript?

apsdehal added a commit to apsdehal/jquery-mobile that referenced this issue May 7, 2016

apsdehal added a commit to apsdehal/jquery-mobile that referenced this issue Jun 23, 2016

Member

apsdehal commented Jun 23, 2016

Landed in 1.5-dev.

@apsdehal apsdehal closed this Jun 23, 2016

arschmitz added a commit to arschmitz/jquery-mobile that referenced this issue Jul 4, 2016

arschmitz added a commit that referenced this issue Jul 4, 2016

@apsdehal apsdehal self-assigned this Aug 3, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment