0

I'm trying to create a page that changes every 10 seconds 4 times and then begins again. To do this I made a counter and passed it along with my function. It doesn't even seem like its loading.

I tried using <body onload="start()"> as well.

<script>
var i = 1;

function start(){
    i = setInterval(changeEdu, 10000, i);
}

function changeEdu(i){  
    if(i == 4){
        i = 1;
    }else{
        i++;
    }

    document.getElementById("edu1").src = "left" + i + ".jpg";
    document.getElementById("edu2").src = "right" + i + ".jpg";

    return i;
}

</script>
Chin Leung
  • 14,621
  • 3
  • 34
  • 58
  • 1
    [Notice that the return value from `setInterval` is the *interval ID*](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setInterval). You're trying to read the interval ID like a counter. –  Feb 07 '18 at 19:48
  • 3
    `i = setInterval` this is not what you think it is. `setInterval` returns an ID, so when you call `start()`, i is immediately set to whatever the ID of the interval is, and you increment from there. – Sterling Archer Feb 07 '18 at 19:48

3 Answers3

2

By declaring i as a parameter of your function, your increment will only mutate the local variable and not your global state. Also the return value is ignored.

var i = 1;

function start() {
    setInterval(changeEdu, 10000);
}

function changeEdu() {
//                ^^
    if (i == 4) {
        i = 1;
    } else {
        i++;
    }

    document.getElementById("edu1").src = "left" + i + ".jpg";
    document.getElementById("edu2").src = "right" + i + ".jpg";
}
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • This isn't great because it pollutes the global scope with a variable called i. – Demonblack Feb 07 '18 at 19:54
  • 1
    @Demonblack It also pollutes the global scope with variables called `start` and `changeEdu` - it shouldn't be much of a problem. Maybe a more descriptive name, like `currentEdu`, might be better, but variable scope wasn't the topic of this question. One can of course use the [module pattern](https://stackoverflow.com/questions/592396/what-is-the-purpose-of-a-self-executing-function-in-javascript) to avoid issues. – Bergi Feb 07 '18 at 19:57
  • I have more of an issue with i because it's so common. I know variable scope isn't the primary issue here, but this is clearly a newbie and we should give him something that isn't a worst practice, especially when it's so easy. – Demonblack Feb 07 '18 at 20:00
0

I believe it is because you are reassigning variable i at the onset of the start() function. setInterval returns a number that is used to cancel the function with the clearInterval(theNumber) function.

I am also quite new to JS, but I would try to delete the reassignment in the start() function and try again.

cvr
  • 33
  • 5
0

That is not how setInterval works, it doesn't return your return value. You need to create a closure.

Also, your startInterval() is never called. Change it to this and it works:

<script>
(function(){
    var i = 1;
    function changeEdu(){  
        if(i == 4){
            i = 1;
        }else{
            i++;
        }

//      document.getElementById("edu1").src = "left" + i + ".jpg";
//      document.getElementById("edu2").src = "right" + i + ".jpg";
console.log(i);
    }

    setInterval(changeEdu, 10000);
})();
</script>
Demonblack
  • 384
  • 2
  • 8