1

First of all, this code, as ugly as it is, works, for all intents and purposes. However, it really makes me both sick and mad to look at.

For the lazy, the main portion of the code that sucks is this:

setTimeout(function() {
    toggle(); //off
    setTimeout(function() {
        toggle(); //on
        setTimeout(function() {
            toggle(); //off
            setTimeout(function() {
                toggle(); //on, stays for stallTime
                setTimeout(function() {
                    toggle(); //off
                    callback();
                }, stallTime);
            }, 300);
        }, 300);
    }, 300);
}, 300);

I would, of course, love to control my animations just by changing the CSS class on an element. However, transitionend and webkitTransitionEnd and the like are not reliable in the least. Am I forced to resorting to this kind of hideous code? Ideally I'd like to be able to set a variable number of "flashes", but this code is too stodgy for that.

Please rescue me from my filth.

Jason
  • 51,583
  • 38
  • 133
  • 185
  • Why don't you set a loop to do this instead...? – jeremy Dec 28 '12 at 20:13
  • because it needs to be timed, and the css is controlling the transition timing. a loop would just fire it a bunch of times without waiting. – Jason Dec 28 '12 at 20:15
  • not if you do something like [this](http://jsfiddle.net/6RDJk/) – jeremy Dec 28 '12 at 20:16
  • That's gratuitous and unnecessary (and kinda funny). Are you familiar with Common.js Deferred objects? jQuery has them (and the subset Promise). Check it out. Funny, this one particular pattern (deferred objects/promise/futures) keeps coming up today. – Jared Farrish Dec 28 '12 at 22:45
  • Yes, I'm familiar with deferreds. The problem is, this is a timing issue that requires a timeout or interval. – Jason Dec 28 '12 at 23:23
  • @Jason - This is a demo showing how you could use one. It's a little trickier to keep it wholly self-contained (with scope and all), but it's not impossible to only use one `setTimeout`. – Jared Farrish Dec 29 '12 at 00:19

2 Answers2

3
var count = 5;
var i = setInterval(function() {
    toggle();
    count--;
    if(count == 0) {
        clearInterval(i);
        callback();
    }
}, 300);
Dark Falcon
  • 43,592
  • 5
  • 83
  • 98
  • brilliant. this is an obvious answer that i should have known. i blame it on the holidays. thank you! – Jason Dec 28 '12 at 20:24
  • Nile's is shorter though. :) – Dark Falcon Dec 28 '12 at 20:26
  • I'd also recommend it because it doesn't use intervals. You could also use a recursive function. – jeremy Dec 28 '12 at 20:28
  • Is there a problem with using intervals? – Dark Falcon Dec 28 '12 at 20:28
  • Intervals, in Javascript, are infamous for being evil. They can get laggy and are added to a queue when first defined so that the browser is constantly trying to "catch up." You can find out more [here on SO](http://stackoverflow.com/questions/7142192/is-setinterval-and-settimeout-bad-things-to-do-in-modern-jquery-animations) or [here on another link I found](http://wallofscribbles.com/2011/setinterval-the-sneaky-basterd-child-of-javascript/). In general, you'd have more control over your code and have assurance that its running smoother if you use timeOuts instead. :D – jeremy Dec 28 '12 at 20:32
  • 1
    Thanks. The first link was instructive. The second link seemed to be written by someone who misunderstands the basic concept of variable manipulation in Javascript, so not quite so helpful. :) – Dark Falcon Dec 28 '12 at 20:36
  • i like the interval method, personally. just turn it off when i don't need it anymore as opposed to stacking up timeouts of various lengths. thank you both for your contributions though. – Jason Dec 28 '12 at 20:36
1

To avoid setInterval, I wrote a recursive function:

// link.addClass('hover'); //on - no longer needed

flash = function(count) {
    if (count > 1) {
        setTimeout(function() {        
            toggle();
            flash(count - 1);
        }, 300);
    } else {
        setTimeout(function() {
            toggle(); //off
            callback();
        }, stallTime);            
    }                
}

flash(6); // 2 * the number of flashes.
CAbbott
  • 8,078
  • 4
  • 31
  • 38
  • @JaredFarrish No... CAbbott's code doesn't do what DarkFelcon's did. If DarkFelcon's did the same, he'd also need two `setInterval`s (or one `setInterval` and one `setTimeout`). All CAbbott did was make it exactly what the OP was originally asking for. DarkFelcon didn't, if you look close enough. There's no `stallTime` timer in the accepted answer. – jeremy Dec 28 '12 at 23:44
  • @Nile - I don't think you *require* two separate `setTimeout`s to get that done. Is there something you can indicate to help me see what I'm missing? There aren't two `setTimeout`s running at once. So...? – Jared Farrish Dec 28 '12 at 23:50
  • 1
    @JaredFarrish Something like [this](http://jsfiddle.net/KDSRX/4/) would do the same with only one timeout. Also [this](http://jsfiddle.net/KDSRX/5/) if you don't feel like cluttering the global scope – jeremy Dec 28 '12 at 23:59
  • @Nile - This was a [contrived example](http://jsfiddle.net/userdude/NX43d/3/) of a single `setTimeout` being used for a [question earlier today](http://stackoverflow.com/questions/14070010/javascript-force-new-loop-iteration-in-setinterval/14070344#14070344). I'm not sure why, but I can sit for hours and fiddle with obscure ways of calling `setTimeout`. It's just a matter of how the code is structured, though, and as I said in my comment below the question, it's as funny as it is absurd. – Jared Farrish Dec 29 '12 at 00:08
  • @JaredFarrish I wasn't sure if you were questioning the usage of `setTimeout` and implying that `setInterval` would be more suitable here – jeremy Dec 29 '12 at 00:10
  • @Nile - Oh god no. I'm not a anti-intervalist, it's not as turrible as it's been made out to be (and may even be *better* where appropriate... imagine that). I do like the sense of control that you get when using `setTimeout` too. But no, it was the his-and-hers calls to *two* that made me laugh. That's it. – Jared Farrish Dec 29 '12 at 00:16
  • @Nile - See, here's an earlier fight with trying to make it more elegant. Not quite right... http://jsfiddle.net/userdude/NV7HU/2 `looper()` at the bottom bothered me. – Jared Farrish Dec 29 '12 at 00:30
  • @Nile - Ah ha! I've been looking for this one for a while. http://stackoverflow.com/a/12400216/451969 Wrap your head around that one. Yeesh. – Jared Farrish Dec 29 '12 at 00:37
  • You can update the post to my code if you want. @JaredFarrish - woah – jeremy Dec 29 '12 at 00:41
  • @Nile - Yeah, that was my white whale for a while. Learned a *lot* about function scope. – Jared Farrish Dec 29 '12 at 00:45