javascript - bad algorithm firing concurrent loops

Question

I created a relatively small dynamic banner rotation script with icons at the bottom for bringing a particular banner into focus. Firing a mouseenter over a banner pauses the show, but sometimes when I mouseout from my banner, the delay for certain banners gets shortened. I'd even understand if it just happened once, but the delay is then set for that shorter amount of time every time the banner comes back around in the rotation, and often the shortening happens in one other place in the list of banners, as well. Sometimes this can be corrected by an as yet undetermined set of actions. I'm starting to suspect that my logic is catching the loop in the middle somewhere and so the process branches out, runs two loops, which appear to speed up the calling of the showNextBanner function. Not sure how to solve this. I've put in tests to see if it's currently in play mode, to no avail.

I include what I think are the relevant parts of the code below.

        var firstRun = true;
        var play = true;
        var current = 0;

        var banners = $$( '.banner' ); 
        banners.invoke( 'hide' );
        var images = $$( '.image' );
        var icons = $$( '.icon' );
        //dynamically clones an initial icon to match the number of banners
        initIcons();

        banners.invoke( 'observe', 'mouseenter', function( field ) {
            play = false;
        });

        banners.invoke( 'observe', 'mouseleave', function( field ) {
            if( !play ) {
                play = true;
                showNextBanner().delay(3);
            }
        });

        icons.invoke( 'observe', 'click', function( field ) {
                play = false;
                hideBanner( current );
                showBanner( findObj( icons, field.findElement()));
        });


        showNextBanner().delay(3); 


        function hideBanner( which ) {
            icons[ which ].src = blankIconSRC;
            banners[ which ].hide();
        }


        function showBanner( which ) {
            icons[ which ].src = selectedIconSRC;
            banners[ which ].show();
            current = which;
        }


        // loops the hiding and showing of icons 
        // (mouseenter sets play to false)
        function showNextBanner() {
            if( play ) {
                if( !firstRun ) {
                    if( ++current == banners.length ) current = 0;    
                    var previous = 0;
                    ( current == 0 )? previous = banners.length - 1: previous = current - 1;
                    hideBanner( previous );
                } else {
                    icons[0].src = selectedIconSRC;
                    firstRun = false;
                }
                showBanner( current );
                showNextBanner.delay(3);
            }
        }
    }());

---

After all that, the client wants a jQuery solution so he can have a slide-in effect not available via scriptaculous. So all that work is down the drain. The good news is that I can just use jCarousel, probably, and tweak the stylesheet. Thanks for the help!

Answer 1

I suspect what is happening is that you've got multiple .delay calls stacking up. So you've got one with less than 3 seconds remaining and showNextBanner is called again, setting another delay timer.

As I read the docs, it appears .delay is intended to put gaps in the jquery event pipeline, rather than actually delay function calls. You may benefit from switching to calling setTimeout instead of delay, so that you get a handle to the timeout, which you can then cancel before setting a new timeout (or cancel if play is set to false, then reset when play is true again) This is mentioned in the JQuery docs for .delay

Answer 2

My guess is that since you don't "cancel" the delay()'ed function, they hang around for too long, but they don't do anything when they fire, because play is false. But once play is true again, the all start having an effect again.

You can save the returned value for delay() and cancel the timer by using clearTimeout() with the value.

However, I'd also suggest that you use a single container for all the banners (and maybe put the the icons in there too), and set the mouseenter/mouseleave events on that, rather than on individual banners. Then there's just a single element that'll start/stop the banner rotation. If you also split everything up in specific functions that play and stop the rotation, and one to show a specific banner, you can possibly get a cleaner code structure.

Here's an example (it's just something I put together for fun rather than an edit of your code, so it's quite different. Sorry. But hopefully you can still use for something)