HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Traffic light sequence

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
trafficsequencelight

Problem

I have create a functioning automated traffic light sequence using an array and if statements. It all work correctly but I am wondering if there is anything more I can do to improve my code without changing to structure or way it works, so without the use of dictionaries, etc.



`

Traffic Light


.rainbow {
background-image: -webkit-gradient( linear, left top, right top, color-stop(0, red), color-stop(0.1, yellow), color-stop(0.2, green));
background-image: gradient( linear, left top, right top, color-stop(0, #f22), color-stop(0.15, #f2f), color-stop(0.3, #22f), color-stop(0.45, #2ff), color-stop(0.6, #2f2),color-stop(0.75, #2f2), color-stop(0.9, #ff2), color-stop(1, #f22) );
color:transparent;
-webkit-background-clip: text;
background-clip: text;
}


Traffic Light


Your browser does not support the HTML5 canvas tag.





var c = document.getElementById("myCanvas");
var ctx = c.getContext("2d");

ctx.rect(0, 0, 200, 300);
ctx.fillStyle = "grey";
ctx.fill();

var colours=["red", "yellow", "green", "black","red yellow"];
var current=colours[0];

function offlight() {
ctx.beginPath();
ctx.arc(95,50,40,10,12*Math.PI);
ctx.fillStyle = "black";
ctx.fill();
ctx.stroke();
}

function offlight1() {
ctx.beginPath();
ctx.arc(95,150,40,10,12*Math.PI);
ctx.fillStyle = "black";
ctx.fill();
ctx.stroke();
}

function offlight2() {
ctx.beginPath();
ctx.arc(95,250,40,10,12*Math.PI);
ctx.fillStyle = "black";
ctx.fill();
ctx.stroke();
}

function drawLight1() {
ctx.beginPath();
ctx.arc(95,50,40,10,12*Math.PI);
ctx.fillStyle = "red";
ctx.fill();
ctx.stroke();
}

function drawLight2() {
ctx.beginPath();
ctx.arc(95,150,40,10,12*Math.PI);
ctx.fillStyle = "yellow";
ctx.fill();
ctx.stroke();
}

function drawLight3() {
ctx.beginPath();
ctx.arc(95,250,40,10,12*Math.PI);
ctx.fillStyle = "green";
ctx.fill();
ctx.stroke();

Solution

Ah, you moved your ` into the . Still, what exactly are you looking for? File size (--> code reduction)? Speed optimization?

One simple thing would be using
parameters` to reduce code duplication:

var c = document.getElementById("myCanvas");
var ctx = c.getContext("2d");

ctx.rect(0, 0, 200, 300);
ctx.fillStyle = "grey";
ctx.fill();

var colours = ["red", "yellow", "green",  "red yellow"];
var current = colours[0];

function offlight(a1) {
    ctx.beginPath();
    ctx.arc(95, a1, 40, 10, 12 * Math.PI);
    ctx.fillStyle = "black";
    ctx.fill();
    ctx.stroke();
}

function drawLight(a1, fillParam) {
    ctx.beginPath();
    ctx.arc(95, a1, 40, 10, 12 * Math.PI);
    ctx.fillStyle = fillParam;
    ctx.fill();
    ctx.stroke();
}

function changelight() {
    if (current == colours[0]) {
        drawLight(50, "red");
        offlight(150);
        offlight(250);
        current = colours[4]
    } else if (current == colours[4]) {
        drawLight(50, "red");
        drawLight(150, "yellow");
        offlight(250);
        current = colours[2]
    } else if (current == colours[2]) {
        offlight(50);
        offlight(150);
        drawLight(250, "green");
        current = colours[1]
    } else if (current == colours[1]) {
        offlight(50);
        drawLight(150, "yellow");
        offlight(250);
        current = colours[0]
    }

}
setInterval(changelight, 1000);


Do you ever use colours? ("yellow")

You could also remove the whitespaces and shorten variable and function names. Also think about moving your JS into a separate file.

Here is a working fiddle.

Code Snippets

var c = document.getElementById("myCanvas");
var ctx = c.getContext("2d");

ctx.rect(0, 0, 200, 300);
ctx.fillStyle = "grey";
ctx.fill();

var colours = ["red", "yellow", "green",  "red yellow"];
var current = colours[0];

function offlight(a1) {
    ctx.beginPath();
    ctx.arc(95, a1, 40, 10, 12 * Math.PI);
    ctx.fillStyle = "black";
    ctx.fill();
    ctx.stroke();
}

function drawLight(a1, fillParam) {
    ctx.beginPath();
    ctx.arc(95, a1, 40, 10, 12 * Math.PI);
    ctx.fillStyle = fillParam;
    ctx.fill();
    ctx.stroke();
}

function changelight() {
    if (current == colours[0]) {
        drawLight(50, "red");
        offlight(150);
        offlight(250);
        current = colours[4]
    } else if (current == colours[4]) {
        drawLight(50, "red");
        drawLight(150, "yellow");
        offlight(250);
        current = colours[2]
    } else if (current == colours[2]) {
        offlight(50);
        offlight(150);
        drawLight(250, "green");
        current = colours[1]
    } else if (current == colours[1]) {
        offlight(50);
        drawLight(150, "yellow");
        offlight(250);
        current = colours[0]
    }

}
setInterval(changelight, 1000);

Context

StackExchange Code Review Q#119646, answer score: 4

Revisions (0)

No revisions yet.