0

Basically my code below works but there is a problem to it which I recently discovered. As I was running it the pictures seem to come in the wrong order. I ran it with google chrome and using inspect element I found the order it went in was image1>image4>image1>image2>image4>image1 and so on in order but the starting order is not right and I cant seem to find the source of the issue and running it with google chromes html console cant seem to find the problem.

<html>
<body>

<p>Click the button to change to the next light.</p>

<img id="starting_light" src="image1.jpg">

<button type="button" onclick="nextLightClick()">Next Light</button>

<script>

var lights = new Array("image2.jpg","image4.jpg","image1.jpg");



var index = 0;
var lightsLen = lights.length;

function nextLightClick() {
    index++;

    if (index == lightsLen) 
        index = 0;

    var image = document.getElementById('starting_light');
    image.src = lights[index];
}
</script>

</body>
</html>
Improve this question
1
  • Why did you edit your question to this? What does this have to do with anything? Commented Jan 23, 2017 at 21:32

2 Answers 2

Reset to default
2

It's not an anomaly, you're increasing your index counter before you run the rest of your function so the first image is at the [1] index instead of [0].

Move it to the end of the function and it works as you'd expect.

function nextLightClick() {
    if (index == lightsLen) 
        index = 0;

    var image = document.getElementById('starting_light');
    image.src = lights[index];

    index++;
}

https://jsfiddle.net/vju0sy4w/1/

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

0

Well arrays start at index 0, and you're incrementing your counter immediately, so the index will start at 1 (the 2nd item). Try this instead:

(function() {
  var srcIndex = -1, // Start at -1 since you're incrementing right off the bat (first index will be zero)
      lightsLen = lights.length,
      image = document.getElementById('starting_light');

  // If this needs to be global
  window.nextLightClick = function() {
    srcIndex++;

    if (srcIndex === lightsLen) { 
      srcIndex = 0;
    }

    image.src = lights[srcIndex];
  };
})();
Improve this answer

8 Comments

Incrementing afterwards would eliminate the dependency on an unnecessarily broader variable.
A "broader" variable? It's the same thing except for how it's initialized. You still need the index variable, which I just renamed srcIndex. There's literally no difference except personal preference.
Your variable was declared outside of the function that truly cares about it. Scoping matters, even if it's personal preference.
Are you talking about the image variable? Yes, I declared it within a private scope outside of that function because other functions may want to use it, and it's bad practice to perform the same DOM lookup multiple times. Cache it!
No, I'm talking about srcIndex. It has no earthly business outside of that anonymous function.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.