2

I have this function which acts as a loading box using setInterval to change the background images which creates a flashing effect.

 function loading() {
    clearInterval(start);

    var i = 0;
    function boxes() {

        in_loading = ".in_loading:eq("  + i + ")";
        $(".in_loading").css("background", "url(images/load_bar_green.png)  no-repeat");    
        $(in_loading).css("background", "url(images/load_bar_blue.png)  no-repeat");        

        if(i == 3) {
            i = 0;
        } else {
            i++;
        }

    }
    var start = setInterval(function() {
        boxes();
    }, 350);
}

But even with clearInterval if I click on it more than once the flashing goes out of order. I tried removing the boxes, hiding them but I can't seem to get the 'buffer' cleared? Any ideas?

pimvdb
  • 151,816
  • 78
  • 307
  • 352
Technotron
  • 21
  • 2
  • 4
  • 1
    `clearInterval` and `setInterval` are javascript functions not jquery. jquery is framework that sits on top of javascript. – Eonasdan Oct 12 '11 at 14:24

4 Answers4

3

The reason why it keeps flashing is because every time loading gets called it creates a new variable start, so clearInterval is actually doing nothing. You also shouldn't have the boxes function within loading because it is doing the same thing, creating a new boxes function every time loading is called. This will add a lot of lag the longer the script executes.

var i = 0;
var start;
function loading() {
    clearInterval(start);

    start = setInterval(function() {
        boxes();
    }, 350);
}

function boxes() {

    var in_loading = ".in_loading:eq("  + i + ")";
    $(".in_loading").css("background", "url(images/load_bar_green.png)  no-repeat");    
    $(in_loading).css("background", "url(images/load_bar_blue.png)  no-repeat");        

    if(i == 3) {
        i = 0;
    } else {
        i++;
    }

}
locrizak
  • 12,192
  • 12
  • 60
  • 80
  • It was perfectly find to have the `boxes()` function inside the loading function. There was no need to move it outside and doing so pollutes the namespace more than required. The key to solving this was moving the i and start values to a higher scope. – jfriend00 Oct 12 '11 at 15:01
  • Well ideally you would have everything within one namespace or in a closure so the global namespace isn't polluted. To me this way is easier to understand and cleaner – locrizak Oct 12 '11 at 15:09
  • I was just saying that the OP had `boxes()` inside of `loading()` and you moved it outside to the top level scope. That part of your change was not required and is not desirable and doesn't make it cleaner. – jfriend00 Oct 12 '11 at 15:11
  • Its just a personal preference. I won't put a function within a function unless the inner function is returning something. The global scope will not get polluted because all of this should be in a namespace or a closure. – locrizak Oct 12 '11 at 15:33
  • 1
    OK, I have a different personal preference. I don't put functions at a higher scope than needed. If they're only used within a function, then I declare them there. I think it makes the code more self documenting too since the way the code is declared shows that `boxes()` is a local function not called from anywhere else. I don't know why a return value has anything to do with your choice. – jfriend00 Oct 12 '11 at 15:36
  • I guess the reason why I do this is because, correct me if I'm wrong (maybe a link?), the inner function will be recreated every time `loading` is called? – locrizak Oct 12 '11 at 15:42
1

Function declarations get "hoisted" to the top of their scope, this is what is messing the execution order. Check this: http://javascriptweblog.wordpress.com/2010/07/06/function-declarations-vs-function-expressions/

beefyhalo
  • 1,691
  • 2
  • 21
  • 33
0

The reason is every time you call loading it creates a new Interval or new var start. So if you click it twice, then you have two things manipulating the same data. So you need to have the var start outside of the function and the clearInterval inside. So every time you call loading it clears the interval and creates a new one.

var i = 0;
var start;
function loading() {
    clearInterval(start);

    start = setInterval(boxes, 350);
}

function boxes() {

    in_loading = ".in_loading:eq("  + i + ")";
    $(".in_loading").css("background", "url(images/load_bar_green.png)  no-repeat");    
    $(in_loading).css("background", "url(images/load_bar_blue.png)  no-repeat");        

    if(i == 3) {
        i = 0;
    } else {
        i++;
    }
}
brenjt
  • 15,997
  • 13
  • 77
  • 118
-3

maybe you should take a look at this Jquery Plugin , it seems to manage intervals very well .

Jquery Timers Plugin

Genjuro
  • 7,405
  • 7
  • 41
  • 61