Sean Cobb Sean Cobb - 7 months ago 22
Javascript Question

NaN return value from function Javascript || Function Execution Order

Newbie in Javascript here, and after hours digging trough other questions I'm not quite sure how to explain this to be honest, but i'll give it my best, hopefully you'll be able to help me.

HTML:

<div id='header'> <h1> Pastel Land </h1> </div>

<div id='container'>

<div id='readyContainer'>

<h3> This game will start in </h3>
<h1 id='readySeconds'> </h1>

</div>

<div id='shape'> </div>

</div>

<div id='features'>

<button id='start'> START </button>
<button id='stop'> STOP </button>

<p id='timeBox'></p>
<p id='timeAverageBox'></p>
</div>

<div id='testbox'> </div>


FULL SCRIPT:

document.getElementById('start').onclick = function () {
document.getElementById('readyContainer').style.display = 'block';
document.getElementById('readySeconds').innerHTML = '3'
setTimeout(function() {document.getElementById('readySeconds').innerHTML = '2'}, 1000);
setTimeout(function() {document.getElementById('readySeconds').innerHTML = '1'}, 2000);


setTimeout(readyAlert,3000);
setTimeout(displayShape, 3000);


var style = document.getElementById('shape').style;

var el = document.getElementById('shape');

el.addEventListener("click", a, false);
el.addEventListener("click", b, false);

function a() {

style.display = "none";
displayShapeDelay(); // calls the delay function
}

function b() {

end = new Date().getTime(); // saves time when clicked

var time = (end - start)/1000 ; // calculates interval from shape creation until click

document.getElementById('timeBox').innerHTML = time + 's';

return time;

}

document.getElementById('testbox').innerHTML = b();


function readyAlert() {
document.getElementById('readyContainer').style.display = 'none';

}

function getRandomColor() {

var hex = ["#96ceb4", "#ffeead", "#ff6f69", "#ffcc5c", "#88db8b0", "#528491"];

var color = hex[Math.floor(Math.random() * 6)]; // generates integer numbers [0,5], selects indexed item from hex

return color;

}



function displayShape () {

var percentages = [];

for (var i=0; i<4; i++){ // generates a list with 4 different random integer values [5,60]

percentages.push((Math.floor(Math.random() * 61) + 5));

}

var width = (Math.floor(Math.random() * 61) + 5); // generates integer numbers [5,60]

var shapeRand = Math.random()

if (shapeRand < 0.3) { // circle
style.borderRadius = "50%";

} else if (shapeRand >= 0.3 && shapeRand < 0.6) { // random shape
style.borderTopLeftRadius = percentages[0] + "%";
style.borderBottomRightRadius = percentages[1] + "%";
style.borderTopRightRadius = percentages[2] + "%";
style.borderBottomLeftRadius = percentages[3] + "%";
} else { // square
style.borderRadius = "0%";
}

//general shape styles
style.width = width + "%";
style.height = width + "%";
style.display = "block";
style.backgroundColor = getRandomColor();
style.top = percentages[0] + "%";
style.left = percentages[3] + "%";

start = new Date().getTime(); // saves time when shape is created

console.log(width);
console.log(getRandomColor());
console.log(shapeRand);
console.log(percentages);


}

function displayShapeDelay () { // calls the main function with a delay between ]0s,2s[
setTimeout(displayShape, Math.random() * 2000);
}


document.getElementById('stop').onclick = function() {


}

}


Before I had this:

My goal was to return var 'time' to the global scope, so that I could use it to create an array of each value created with each click. I've realised that this was not possible from an anonymous function.

document.getElementById('shape').onclick = function() { // calls the delay function

style.display = "none";

displayShapeDelay();

end = new Date().getTime();

time = (end - start)/1000 ;

document.getElementById('timeBox').innerHTML = time + 's';

return time

}


So this is the code I have now :

var shapeClick = document.getElementById('shape');

shapeClick.addEventListener("click", a, false);
shapeClick.addEventListener("click", b, false);

function a() {

style.display = "none";
displayShapeDelay(); // calls the delay function
}

function b() {

end = new Date().getTime(); // saves time when clicked

var time = (end - start)/1000 ; // calculates interval from shape creation until click

document.getElementById('timeBox').innerHTML = time + 's';

return time;

}

document.getElementById('testbox').innerHTML = b();


Now, there's a couple of issues with this:

1- I can't seem to understand why the two "time divs" are assigned values after pressing the Start button. This means that function b is running, but shouldn't it only be running after the onClick event?

2- In the 'first-round' I understand why both values show up as NaN, since there's no value assigned to the variable "time" yet. But after the onClick event executes, the 'time' value assigned inside 'timeBox' works fine, but the one called outside the function doesn't. Isn't "return time" inside function b, supposed to be returning the value of the "time" variable?

Thanks in advance!

Answer

Pastel Land

The onclick function is serving as the outer function of a whole lot of code which execute when the start button is clicked. Also all of it runs every time the start button is clicked: click start multiple times quickly to see a problem. .

Inside #start.onclick() you have

document.getElementById('testbox').innerHTML = b();

in mainline click handler code: it is not inside another function and runs when the start button is clicked. Since end has not been set yet, the result for time is NaN. The code inside function b set the content of #timebox as well.

If you run the code in strict mode the javascript engine will tell you that end has not been declared. It should be - even if requred in global scope.

As an aside, fyi, Date.now() avoids the need to create and discard a Date object and is equivalent to new Date().getTime().

I suggest reworking the code to move the logic of Pastel Land outside the start button click handler, and have the click handler call into the main application code as needed but only contain logic specific to the start action itself. If you want to avoid polluting global scope you can include all the code in an IIFE (immediately invoked function expression) that would serve the same scope containment provision that the click handler is currently providing. Kindly, I think the code in its current state is presenting an x-y problem :-)


The Game

A restructured version of Pastel Land is shown below for several reasons: you've had time to try it yourself, most of the code is yours and the remainder demonstrates what was meant by the suggestion. And it's a very silly game that deserves to be played!

<!DOCTYPE html>
<html>
<head>
<title>PastelLand</title>
<meta charset="utf-8">
<script>

window.addEventListener("load", function() // an IIFE
{"use strict"

   // Pastel Land

    var running = false;
    var start = 0;
    var end = 0;
    var times = []; // calculating average still to do
    var el, style;

    function getRandomColor() {
        var hex = ["#96ceb4", "#ffeead", "#ff6f69", "#ffcc5c", "#88db8b0", "#528491"];
        var color = hex[Math.floor(Math.random() * 6)]; // generates integer numbers [0,5], selects indexed item from hex
        return color;
    }

    function displayShape () {
        var percentages = [];

        for (var i=0; i<4; i++){ // generates a list with 4 different random integer values [5,60]
            percentages.push((Math.floor(Math.random() * 61) + 5));
        }
        var width = (Math.floor(Math.random() * 61) + 5); // generates integer numbers [5,60]
        var shapeRand = Math.random()

        if (shapeRand < 0.3) { // circle
            style.borderRadius = "50%";

        } else if (shapeRand >= 0.3 && shapeRand < 0.6) { // random shape
            style.borderTopLeftRadius = percentages[0] + "%";
            style.borderBottomRightRadius = percentages[1] + "%";
            style.borderTopRightRadius = percentages[2] + "%";
            style.borderBottomLeftRadius = percentages[3] + "%";
        } else { // square
            style.borderRadius = "0%";
        }

        //general shape styles
        style.width = width + "px";
        style.height = width + "px";
        style.position = "absolute"
        style.display = "block";
        style.backgroundColor = getRandomColor();
        style.top = percentages[0] + "%";
        style.left = percentages[3] + "%";

        start = Date.now(); // saves time when shape is created

        console.log(width);
        console.log(getRandomColor());
        console.log(shapeRand);
        console.log(percentages);
    }

    function displayShapeDelay () { // calls the main function with a delay between ]0s,2s[
        setTimeout(displayShape, Math.random() * 2000);
    }

    function readyAlert() {
            document.getElementById('readyContainer').style.display = 'none';
    }

    function userFound() {
        style.display = "none";
        end = Date.now(); 
        var time = (end - start)/1000 ; // calculates interval from shape creation until click
        document.getElementById('timeBox').innerHTML = time + 's';
        displayShapeDelay(); // calls the delay function
        times.push( time); // saves time user took to find shape
    }

    function userStart() {

        if( running)
            return;
        running = true;
        document.getElementById('readyContainer').style.display = 'block';  
        document.getElementById('readySeconds').innerHTML = '3'
        setTimeout(function() {document.getElementById('readySeconds').innerHTML = '2'}, 1000);
        setTimeout(function() {document.getElementById('readySeconds').innerHTML = '1'}, 2000);
        setTimeout(readyAlert,3000);
        setTimeout(displayShape, 3000);
        times.length = 0;  // reset times array
    }

    function userStop() {
        running = false;
        style.display="none"
    }
    function init() {
        el = document.getElementById('shape');
        style = el.style;
        el.addEventListener("click", userFound, false);
        document.getElementById('start').onclick=userStart;
        document.getElementById('stop').onclick=userStop;    
    }
    return init; // window load listener
}());
</script>
</head>
<body>
<div id='header'> <h1> Pastel Land </h1> </div>
<div id='container'>
    <div id='readyContainer'>
        <h3> This game will start in </h3>
        <h1 id='readySeconds'> </h1>
    </div>
    <div id='shape' style="height:40px;width:40px;"></div>
</div>
<div id='features'>
    <button id='start'> START </button> 
    <button id='stop'> STOP </button>
    <p id='timeBox'></p>
    <p id='timeAverageBox'></p>
</div>
<div id='testbox'> </div>
</body>
</html>

Calculation and display of averages and minor changes to page presentation remain to be done (I'm not coding them!). Units for width and height were changed from "%" to "px" (pixels) and "position: absolute;" added to shape.style. Functions a and b were combined into function userFound. Code involving "testbox" was omitted. The IIEF returns an initialization function to be executed after window load.


Notes

userStart and userStop click event handlers were defined using named function declarations instead of coding them as anonymous functions in the parameter lists of calls to other functions.

Declared function names refer to the function object created by their declaration. Hence setting the value of an element's onclick attribute to a function name works because the attribute requires a function object value. Setting onclick to the undefined value returned from calling one of the handlers would not work.

The start/stop handlers could have been registered as "click" event listeners, using addEventListener instead of element onclick values.

The init function is not called by the IIFE. The IIFE itself is called when the

window.addEventListener("load", function() { // IIFE code }() );

statement is executed in the head section of the page. All function objects declared at top level within the IIFE are created at this time. The function returned by the IIFE, init, is registered as a listener for later execution, after all DOM elements have been created for the HTML body and the window load event fires.

If you call userStart from init after other el and style have been initialized, the game will start. Although userStart is normally called in response to clicking the start button, it will still behave the same if called by other means.