Why does my code always select the same Array item?

Question

I'm a student having trouble with my HTML code. My task is to create a traffic light sequence using the array in my code, which advances the traffic light sequence after each click of the button. It does this by manipulating a single image on the web page.

However, after pressing the button, it only changes the lights to their 'RedAmberLight' state. The code has been pasted below and any help would be much appreciated:

<!DOCTYPE html>
<html>
<body>

<h1>Changing Traffic Lights w/ Arrays</h1>

<img id="trafficLight" src="RedLight.jpg">
<button type="button" onclick="lightChange()">Change Traffic Lights</button>

<script>

var fileArray = ["RedLight.jpg",
                 "RedAmberLight.jpg",
                 "GreenLight.jpg",
                 "AmberLight.jpg"];

function lightChange() {
  var lightColour = trafficLight.src

  if (lightColour = fileArray[0]) {
     lightColour = fileArray[1];
  } else if (lightColour = fileArray[1]) {
     lightColour = fileArray[2];
  } else if (lightColour = fileArray[2]) {
     lightColour = fileArray[3]; 
  } else {
     lightColour = fileArray[0];
  }
  var light = document.getElementById('trafficLight');
  light.src = lightColour
}

</script>

</body>
</html>

Firstly, you're relying on elements [being added to the global object](http://stackoverflow.com/questions/3434278/do-dom-tree-elements-with-ids-become-global-variables), which they do, but it's not good practice. Secondly, an elements `src` property returns the entire URL, the absolute path. Thirdly, comparison is done with two or three equal signs. — adeneo, Apr 23 '17 at 14:17
Not what you're asking, but the logic could be simplified if you used a variable to keep track of the current index in the array, and then instead of the whole if/else if/else block you could just say something like `currentIndex++; if (currentIndex === fileArray.length) {currentIndex=0} light.src = fileArray[currentIndex]`. — nnnnnn, Apr 23 '17 at 14:20
I appreciate your feedback. I'll be sure to keep your comments in mind for the future... — Reef L., Apr 23 '17 at 20:23

score 2 · Answer 1 · answered Apr 23 '17 at 14:17

2

The Problem :

if (lightColour = fileArray[0]) {

This is an assignment and not testing.

Use double/tripple = : lightColour == fileArray[0])

answered Apr 23 '17 at 14:17

Royi Namir

144,742
138
468
792

Just wondering, is there any difference between the double and triple equals signs? – Sank6 Apr 23 '17 at 14:20
@SankarshMakam http://stackoverflow.com/questions/359494/which-equals-operator-vs-should-be-used-in-javascript-comparisons – Royi Namir Apr 23 '17 at 14:20

Ted Hopp · Accepted Answer · 2017-04-23T14:19:45.577

2

You need to use the == or === operator rather than = to compare values. Otherwise you are simply assigning the value and testing whether the result is "falsy" or "truthy" (a JavaScript concept that tests for the empty string, 0, etc., or not).

if (lightColour === fileArray[0]) {
   lightColour = fileArray[1];
} else if (lightColour === fileArray[1]) {
   lightColour = fileArray[2];
} else if (lightColour === fileArray[2]) {
   lightColour = fileArray[3]; 
} else {
   lightColour = fileArray[0];
}

edited Apr 23 '17 at 14:19

answered Apr 23 '17 at 14:17

Ted Hopp

232,168
48
399
521

*"whether the result is "truthy" (a JavaScript concept that tests for the empty string, 0, etc.)"* - Those examples are falsy values... – nnnnnn Apr 23 '17 at 14:18
@nnnnnn - Oops. Yeah, I phrased that poorly. – Ted Hopp Apr 23 '17 at 14:19
Thank you. I just had to change a few things around and, using your advice, was able to make the code function correctly... – Reef L. Apr 27 '17 at 11:27

Oen44 · Answer 3 · 2017-04-23T20:59:10.400

How about less if statements? Check this out.

var fileArray = [
  "RedLight.jpg",
  "RedAmberLight.jpg",
  "GreenLight.jpg",
  "AmberLight.jpg"
];

function lightChange() {
  var lightColour = trafficLight.innerHTML;
  var lightIndex = fileArray.indexOf(lightColour);
  lightColour = fileArray[ lightIndex+1 == fileArray.length ? 0 : lightIndex+1];

  var light = document.getElementById('trafficLight');
  light.innerHTML = lightColour;
}

<h1>Changing Traffic Lights w/ Arrays</h1>

<div id="trafficLight">RedLight.jpg</div>
<button type="button" onclick="lightChange()">Change Traffic Lights</button>

With img tag, it should look like this. I have used getAttribute('src') rather than src, why? Because src will return entire url like http://localhost/RedLight.jpg.

var fileArray = [ //Array of files name, obviously
  "RedLight.jpg",
  "RedAmberLight.jpg",
  "GreenLight.jpg",
  "AmberLight.jpg"
];

function lightChange() { // That function tho
  var trafficLight = document.getElementById('trafficLight'); // Get element with id 'trafficLight'
  var lightColour = trafficLight.getAttribute('src'); // Get value of attribute 'src' from 'trafficLight' element
  var lightIndex = fileArray.indexOf(lightColour); // Get index of value from fileArray based on actually selected light
  lightColour = fileArray[ lightIndex+1 == fileArray.length ? 0 : lightIndex+1]; // Set new lights from array by checking if new index is equal to array lenghth, if it is, then select first index from array, otherwise go to next value from array by selecting actually selected index+1

  trafficLight.src = lightColour; // Set new src value
}

<h1>Changing Traffic Lights w/ Arrays</h1>

<img id="trafficLight" src="RedLight.jpg">
<button type="button" onclick="lightChange()">Change Traffic Lights</button>

Thanks for the help; this actually got my code to function correctly... Obviously you don't have to, but if you could, I'd be grateful if you could explain what each of the lines you've added do to the code. — Reef L., Apr 23 '17 at 20:16

Why does my code always select the same Array item?

3 Answers3