Scope or function hoisting issue? I am not sure

Question

As below, I made a simple high scores array that is saved to local storage and added to with user prompts.

As an independent file by itself it works great. Or at least it seems to be.

However, when I try to integrate this into my larger application I seem to be having scope issues with my global variable, allScores . The length of the array stays at 0. I checked to see if I have any variable duplicates and I do not.

I've been trying to read about function hoisting and scope. What I am not sure about is why the below code works as an independent file, but when I integrate it into my larger application I have scope issues.

How should I be doing this differently? As I am new to JavaScript my best practices are most likely off. Your guidance is appreciated. Thanks.

var allScores = [];

function saveScore() {

if (allScores.length === 0) {
    allScores[0]= prompt('enter score', '');
    localStorage ['allScores'] = JSON.stringify(allScores);
}

else if (allScores.length < 3) {
    var storedScores = JSON.parse(localStorage ['allScores']);
    storedScores = allScores;
    var injectScore = prompt('enter score', '');
    allScores.push(injectScore);
    allScores.sort(function(a, b) {return b-a});
    localStorage ['allScores'] = JSON.stringify(allScores);
}

else {
    var storedScores = JSON.parse(localStorage ['allScores']);
    storedScores = allScores;
    var injectScore = prompt('enter score', '');
    allScores.pop();
    allScores.push(injectScore);
    allScores.sort(function(a, b) {return b-a});
    localStorage ['allScores'] = JSON.stringify(allScores);
}
document.getElementById('readScores').innerHTML = allScores;
}**

Answer 1

I have refactored your code in an effort to display some practices which may help you and others in the future, since you mentioned best practices in the question. A list of the concepts utilized in this refactoring will be below.

var saveScore = (function () { /* Begin IIFE */
    /*
    The variables here are scoped to this function only.
    They are essentially private properties.
    */
    var MAX_ENTRIES = 3;

    /*
    Move the sorting function out of the scope of saveScore,
    since it does not need any of the variables inside,
    and possibly prevent a closure from being created every time
    that saveScore is executed, depending upon your interpreter.
    */
    function sorter(a, b) {
        return b - a;
    }

    /*
    As far as your example code shows, you don't appear to need the
    allScores variable around all the time, since you persist it
    to localStorage, so we have this loadScore function which
    pulls it from storage or returns a blank array.
    */
    function getScores() {
        var scores = localStorage.getItem('scores');
        return scores ? JSON.parse(scores) : [];
        /*
        Please note that JSON.parse *could* throw if "scores" is invalid JSON.
        This should only happen if a user alters their localStorage.
        */
    }

    function saveScore(score) {
        /* Implicitly load the scores from localStorage, if available. */
        var scores = getScores();

        /*
        Coerce the score into a number, if it isn't one already.
        There are a few ways of doing this, among them, Number(),
        parseInt(), and parseFloat(), each with their own behaviors.

        Using Number() will return NaN if the score does not explicitly
        conform to a textually-represented numeral.
        I.e., "300pt" is invalid.

        You could use parseInt(score, 10) to accept patterns
        such as "300pt" but not anything with
        leading non-numeric characters.
        */
        score = Number(score);

        /* If the score did not conform to specifications ... */
        if (isNaN(score)) {
            /*
            You could throw an error here or return false to indicate
            invalid input, depending on how critical the error may be
            and how it will be handled by the rest of the program.

            If this function will accept user input,
            it would be best to return a true or false value,
            but if a non-numeric value is a big problem in your
            program, an exception may be more appropriate.
            */

            // throw new Error('Score input was not a number.');
            // return false;
        }

        scores.push(score);
        scores.sort(sorter);

        /*
        From your code, it looks like you don't want more than 3 scores
        recorded, so we simplify the conditional here and move
        "magic numbers" to the header of the IIFE.
        */
        if (scores.length >= MAX_ENTRIES) {
            scores.length = MAX_ENTRIES;
        }
        /* Limiting an array is as simple as decreasing its length. */

        /* Save the scores at the end. */
        localStorage.setItem('scores', JSON.stringify(scores));

        /* Return true here, if you are using that style of error detection. */
        // return true;
    }

    /* Provide this inner function to the outer scope. */
    return saveScore;

}()); /* End IIFE */

/* Usage */
saveScore(prompt('Enter score.', ''));

As you can see, with your score-handling logic encapsulated within this function context, virtually nothing could tamper with the interior without using the interface. Theoretically, your saveScore function could be supplanted by other code, but the interior of the IIFE's context is mutable only to those which have access. While there are no constants yet in standardized ECMAScript, this methodology of the module pattern provides a decent solution with predictable outcomes.

• IIFE, an Immediately-Invoked Function Expression, used to create our module
• DRY, Don't Repeat Yourself: code reuse
• Magic numbers, and the elimination thereof
• Type coercion of a string to a number
• Error handling, and discernment between exception or return code use. Related: 8, 9

Answer 2

Have you considered JS closures? Here is some piece to give you an idea..

var scoreboard = (function () {
    var _privateVar = "some value"; //this is never exposed globally
    var _allScores = [];

    return {
        getAllScores: function() { //public
            return _allScores;
        },
        saveScore: function(value) { //public
            _allScores.push(value);
        }
    };
})();

alert(scoreboard.getAllScores().length); //0
scoreboard.saveScore(1);
alert(scoreboard.getAllScores().length); //1
alert(scoreboard._privateVar); //undefined
alert(scoreboard._allScores); //undefined

This way your variables and functions are never exposed to the window object and you don't need to worry about duplicates or scopes. The only variable that has to be unique is the name of your closure function (scoreboard in this example case).

Answer 3

Without having access to your environment, the best thing you can do is get use the firefox dev tools (or get firebug) to put a breakpoint in your saveScore function. You can step through line-by-line and check values and even evaluate expressions within the current scope in a console window(REPL).

https://developer.mozilla.org/en-US/docs/Tools/Debugger - with firefox

http://getfirebug.com/javascript - with firebug(a firefox plugin)

If you're doing web-development, these are priceless resources, so invest some time into learning how to use them.(They will save you much more time down the road!)