1

I'm refactoring some legacy code that uses the revealing module pattern and there are instances of data being stored on the module which are then referenced later within the module's code. It seems like simply storing the data within the module itself in variables would be a more common and readable approach. For instance, the the first example below the getPageLoadState function sets a global property on the main object/module which is then accessed to determine a Boolean.main.pageLoaded is exposed to the client in global scope this way, and I'm not sure what the benefits are. In the second example the getPageLoadState function is removed and the state relies on the pageLoaded variable local to module to determine whether execution of loadData() should get some data or not.

Current set-up:

var main = (function() {
    function init() {
        loadData(); //loads data
        loadData(); //does nothing
    }

    function getPageLoadState() { //exposes module.pageLoaded on first execution and returns boolean
        if (main.pageLoaded !== undefined) { //main.pageLoaded has been set
            return true;
        }
        main.pageLoaded = "true"; //set object 'pageLoaded' on main module to true on first execution of this function so subsequent runs will return true based in the if statement above
        return false; //return false on initial execution
    }

    function loadData() {
        var pageLoaded = getPageLoadState(); //returns false only on first execution
        if (!pageLoaded) {
            console.log("getting data");
        }
    }
    return {
        init: init
    };
})();
main.init();//"getting data"
console.log(main.pageLoaded); //"true"

How I would do it:

var main = (function() {
    var pageLoaded;

    function init() {
        loadData(); //loads data
        loadData(); //does nothing
    }

    function loadData() {
        if (!pageLoaded) { //condition is met only on first run of function when pageLoaded === undefined
            console.log("getting data");
            pageLoaded = true; //subsequent executions will not meet the if clause
        }
    }
    return {
        init: init
    }
})();
main.init();//"getting data"
Stu
  • 324
  • 3
  • 10
  • 1
    Yeah, `pageLoaded` is public, which doesn't appear to have any benefits, other than making the code slightly more unpredictable. I agree with you, I don't see the point - a private module scoped variable makes much more sense. – CertainPerformance Apr 10 '19 at 23:04
  • Can you explain what would make the code unpredictable? Does this refer to risk that some external operator could modify the public value and therefore change the expected behavior of the function? – Stu Apr 18 '19 at 15:41
  • 1
    Right - private properties are easier to reason about than public properties. – CertainPerformance Apr 18 '19 at 18:27

0 Answers0