CODEWITHSUNDEEP
javascriptarraysfunction
Lewis TAYLOR
Lewis TAYLOR

Reputation: 59

Using an array for functions in javascript

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();

Upvotes: 4

Views: 81

Answers (4)

T.J. Crowder
T.J. Crowder

Reputation: 1074168

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.

Upvotes: 1

Yosvel Quintero
Yosvel Quintero

Reputation: 19070

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>

Upvotes: 0

Nina Scholz
Nina Scholz

Reputation: 386560

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;
    }
}

Upvotes: 0

DanBreakingNews
DanBreakingNews

Reputation: 11

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];

Upvotes: 0

Related Questions