Imesh Chandrasiri
Imesh Chandrasiri

Reputation: 5679

setInterval doesn't get fired

Below is the functions I use to run a function periodically. I use the function to change the background of the body. but it doesn't get fired for some reason. Please help me with this code.

setInterval(uiImageChanger(),1);

    function uiImageChanger(){
        var currentTime = new Date().getHours();
        var images = ['image1.jpg','image2.jpg'];

        if( currentTime > 00 && currentTime <= 12){
            $('body').css('background-image', "url(" + randomImagePicker(images ,'breakfast') + ")");
        }else if( currentTime > 12 && currentTime <= 16){
            $('body').css('background-image', "url(" + randomImagePicker(images ,'lunch') + ")");
        }else if( currentTime > 16 && currentTime <= 00){
            $('body').css('background-image', "url(" + randomImagePicker(images ,'dinner') + ")");
        }
    }

    function randomImagePicker(imgArray,time){
        if(time == 'breakfast'){
            return "../images/main_image/breakfast/" + imgArray[Math.floor(Math.random() * imgArray.length)];
        }else if(time == 'lunch'){
            return "../images/main_image/lunch/" + imgArray[Math.floor(Math.random() * imgArray.length)];
        }else if(time == 'dinner'){
            return "../images/main_image/dinner/" + imgArray[Math.floor(Math.random() * imgArray.length)];
        }
    }

Thank you.

Upvotes: 1

Views: 104

Answers (4)

Michael Geary
Michael Geary

Reputation: 28850

You've gotten several answers about the setInterval() problem. I'd like to point out a couple of other problems in the code.

First, this test will always fail:

else if( currentTime > 16 && currentTime <= 00)

After all, if a number is > 16 it cannot also be <= 0.

Also, you may get a warning about 00 being an octal constant which is deprecated. Of course, octal zero is the same value as decimal zero, but watch out for inadvertent octal constants: avoid using a leading zero.

And there is a lot of repetition in the code. You can easily remove all of this repetition to make the code more maintainable. Consider this approach:

// Return a random integer >= 0 and < n
function randomInt( n ) {
    return Math.floor( Math.random() * n );
}

// Return a random element from an array
function randomElement( array ) {
    return array[ randomInt(array.length) ];
}

function uiImageChanger(){
    var hour = new Date().getHours();
    var meal =
        hour <= 12 ? 'breakfast' :
        hour <= 16 ? 'lunch' :
        'dinner';
    var images = [ 'image1.jpg', 'image2.jpg' ];
    $('body').css(
        'background-image',
        'url(../images/main_image/' + meal +
            '/' + randomElement(images) + ')'
    );
}

Upvotes: 1

PSL
PSL

Reputation: 123739

Remove parens from the setInterval function argument. Now, this will invoke the function and set the return value of the function as the reference to setInterval which here is undefined as you don't return anything. So basically you are setting interval on nothing, so nothing happens except the first execution while setting up the setInterval.

Change

setInterval(uiImageChanger(),1); // This will invoke the function immediately.

to

setInterval(uiImageChanger,1); // You want to set the reference of the function to setInterval.

Upvotes: 5

ashoka
ashoka

Reputation: 651

There are multiple ways of defining the function to be executed using setInterval. One of the method is using the function reference for which the example is given by #mohkhan. However you can do the following as well

    setInterval(function(){
           // code comes here.
    }, time_in_mills);

Also I see that you have mentioned the value for the function execution as 1. This means every 1 millisecond the function will be executed, which is not a good practice at all. Give a realistic time in millisecond so that you have given sufficient time for the code to execute.

Upvotes: 1

mohkhan
mohkhan

Reputation: 12295

You have to pass a pointer to a function and not execute the function.

setInterval(uiImageChanger,1);

Upvotes: 4

Related Questions