4

I'm trying to create a program that uses an array of functions to cycle through the order of execution. I've included the program below, can somebody please suggest what i have done wrong.I have created a set of traffic lights on HTML and im trying to write some javascript that will change which light is displayed when a button is clicked. I've created an array of functions that determines the order i want the lights to appear in. I've also written the functions that will display each light. I'm new to javascript, any help would be appreciated.

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
    <title>Task three</title>
    <link href="Task 3-CSS.css" rel="stylesheet" type="text/css" />
    <script src="Task3-Java.js"></script>
</head>

<body>

<div id="control_panel">
    <button onclick="change_light">Change Light</button>
</div>

<div id="traffic_light">
    <div id="red_light" class="light"></div>
    <div id="amber_light" class="light"></div>
    <div id="green_light" class="light"></div>
</div>
</body>
</html>

var light_array=[red,red_amber,green,amber];
var light_index = 0;

function no_light(){
    document.getElementById('red_light').style.backgroundColor = "black";
    document.getElementById('amber_light').style.backgroundColor = "black";
    document.getElementById('green_light').style.backgroundColor = "black";
}

function red(){
    no_light();
    document.getElementById('red_light').style.backgroundColor="red";
}

function red_amber(){
    no_light();
    document.getElementById('red_light').style.backgroundColor="red";
    document.getElementById('amber_light').style.backgroundColor="orange";
}

function green(){
    no_light();
    document.getElementById('green_light').style.backgroundColor="green";
}

function amber(){
    no_light();
    document.getElementById('amber_light').style.backgroundColor="orange";
}

function change_light(){
    light_array[light_index](red,red_amber,green,amber);
    light_index++;
    if (light_index > 3){light_index = 0;}

}
change_light();
9
  • Can you tell us what problem you are having? Do you get an error? Commented Apr 18, 2017 at 13:34
  • You should change your tag sas this doesn't concern Java. Java and Javascript are completely different. Commented Apr 18, 2017 at 13:34
  • 2
    I always wonder why incomplete questions that are not that complex get up voted within seconds of being posted. Commented Apr 18, 2017 at 13:34
  • I suggest you add the relevant HTML and make it a Stack Snippet, so we can test your code right here in your question - check the chapter about minimal reproducible example. Also, you forgot to actually explain how your code does not work or not do what you expect. Commented Apr 18, 2017 at 13:35
  • 2
    I upvoted it, may I? :) For me it's quite complete and clear what the problem OP has. Commented Apr 18, 2017 at 13:37

4 Answers 4

1

You're calling the functions and storing the result in the array. You should just be referring to them (without ()):

var light_array=[red,red_amber,green,amber];

Then call them when you want to call them (e.g., in change_light):

light_array[light_index]();
// ---------------------^

BTW, the code updating light_index is incorrect, you should be incrementing before the if:

function change_light(){
    light_array[light_index]();
    light_index++;                         // <== Moved this up
    if (light_index > 3){light_index = 0;}
}

...but there's a handy trick for that which combines it into one expression:

function change_light(){
    light_array[light_index]();
    light_index = (light_index + 1) % light_array.length;
}

That handles wrap-around for you. Also note how I used light_array.length rather than a hardcoded number, so that if you add/remove entries in the array, the code still works.

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

1 Comment

@YosvelQuintero: Thanks for letting me know, I forgot the parens. Fixed now.
0

I suggest to move all the document.getElementById() to variable declarations.

In the html instead of doing onclick="change_light" you should do onclick="change_light()"

In one line you can handle the light_index variable with light_index = (light_index >= 3) ? 0 : ++light_index;

The rest of the answers has well covered all the other points.

Working example:

var light_array = [red, red_amber, green, amber],
  light_index = 0,
  red_light_elem = document.getElementById('red_light'),
  amber_light_elem = document.getElementById('amber_light'),
  green_light_elem = document.getElementById('green_light');

function no_light() {
  red_light_elem.style.backgroundColor = 'black';
  amber_light_elem.style.backgroundColor = 'black';
  green_light_elem.style.backgroundColor = 'black';
}

function red() {
  no_light();
  red_light_elem.style.backgroundColor = 'red';
}

function red_amber() {
  no_light();
  red_light_elem.style.backgroundColor = 'red';
  amber_light_elem.style.backgroundColor = 'orange';
}

function green() {
  no_light();
  green_light_elem.style.backgroundColor = 'green';
}

function amber() {
  no_light();
  amber_light_elem.style.backgroundColor = 'orange';
}

function change_light() {
  light_array[light_index]();
  light_index = (light_index >= 3) ? 0 : ++light_index;
}
<div id="control_panel">
    <button onclick="change_light()">Change Light</button>
</div>

<div id="traffic_light">
    <div id="red_light" class="light">red</div>
    <div id="amber_light" class="light">amber</div>
    <div id="green_light" class="light">green</div>
</div>

Comments

0

You can try to change this line

var light_array=[red(),red_amber(),green(),amber()];

for this one

var light_array=[red,red_amber,green,amber];

Comments

0

You have three problems (if the rest is working)

  1. using the result instead of function reference

    var light_array = [red, red_amber, green, amber];
    
  2. missing call of the functions

    light_array[light_index]();
    //                      ^^
    

    You stored a reference to the function in light_array. To call a function, you need to invoke it with parenthesis.

  3. use increment before checking

    function change_light() {
        light_array[light_index](); // call function
        light_index++;              // increment counter
        if (light_index > 3) {      // check counter
            light_index = 0;
        }
    }
    

4 Comments

Thanks for the response but can you explain part 2 please?
I've made a few change
changes* but i'm not sure how to call the function
just use the line light_array[light_index](); without any parameters. it works with too, but it makes no sense.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.