Why is this closure not working as I expect it to?

Question

I have the following html and javascript (jQuery):

<div class="container-a">
    <div class="element">...</div>
    <div class="element">...</div>
    ...
</div>
<div class="container-b">
    <div class="element">...</div>
    <div class="element">...</div>
    ...
</div>

<script>
    function cycle($container) {
        setInterval(function() {
            $active = $container.find(':last-child')
            $next = $active.prev();
            $next.css({opacity:0});
            $next.insertAfter($active);
            $next.animate({opacity: 1}, 500, function() {
                $active.insertBefore($container.find(':first-child'));
            });
        }, 3500);
    }

    $(function() {
         cycle($('.container-a'));
         cycle($('.container-b'));
    })
</script>

When I run cycle(..) on just one or the other of .container-a or .container-b things work fine (the elements are faded in one after the next by moving the last element to the beginning of the container after an opacity transition). However, when I run cycle on both as above, then the elements in container-a don't transition properly.

I know that it is because of a closure issue because when I step through the code there are cases when the animation complete function is run and $container is .container-a but $active.parent() and $next.parent() are .container-b. I'm having trouble figuring out why exactly this is the case and how to fix it.

You should use "var" before your variables declaration inside your closure so it keeps them private — Felipe Skinner, Nov 18 '14 at 16:43

score 2 · Accepted Answer · edited May 23 '17 at 12:20

You haven't use var for any of your variable declarations, so they're all implicit globals. You're not using the variable-scoping behavior of closures, because none of your functions have local variables.

Instead, you must use var:

var $active = $container.find(':last-child')
var $next = $active.prev();

Strict mode does not allow implicit globals. If you add "use strict"; to the top of your setInterval callback (or the top of your whole file), you'll see an error that $active is not defined.

See also What is the purpose of the var keyword and when to use it (or omit it)?

score 1 · Answer 2 · answered Nov 18 '14 at 16:34

1

$next is a global variable. Add "var" to the beginning of this line:

$next = $active.prev();

answered Nov 18 '14 at 16:34

TV's Frank

782
7
21

Why is this closure not working as I expect it to?

2 Answers2