2

I was experimenting with Javascript closure and the below code keeps printing 9 always. But as i am creating 10 closures shouldn't previous values persist?

var arr = [];
function getModule(value){
        return (function (_value) {
              var m_val = _value;

              this.getValue = function () {
                  return m_val;
              };

              return {
                ShowValue: function () {
                    console.log(getValue.call(this));
                }
              };
        })(value);
}
for(var i = 0; i < 10 ; i++){
    arr.push(getModule(i))
}
for(var i = 0; i < 10 ; i++){
    arr[i].ShowValue();
}

Please help understand this weird behaviour.

Arup
  • 374
  • 1
  • 10

1 Answers1

4

But as i am creating 10 closures shouldn't previous persist?

You're creating them, but then immediately overwriting them with the next one. this in your anonymous function is a reference to the global object (also available as window on browsers), so each this.getValue = ... overwrites the one before. The reason it's a reference to the global object is that the way you're calling that function doesn't set any specific this, so it defaults to the global object (this is something "strict" mode changes; see more below). More about how this works in this question's answers.

Just use a function declaration, rather than assigning to a property of this:

function getValue() {
    return m_val;
}

Also note that there's no reason to set this when calling it, so:

console.log(getValue.call(this));

becomes

console.log(getValue());

Live Example:

var arr = [];
function getModule(value){
        return (function (_value) {
              var m_val = _value;
              
              function getValue() {
                  return m_val;
              }

              return {
                ShowValue: function () {
                    console.log(getValue());
                }
              };
        })(value);
}
for(var i = 0; i < 10 ; i++){
    arr.push(getModule(i))
}
for(var i = 0; i < 10 ; i++){
    arr[i].ShowValue();
}
.as-console-wrapper {
  max-height: 100% !important;
}

Some other notes:

  • There's no need for that inner anonymous function at all, and no need for getValue; just use value directly:

    function getModule(value){
        return {
          ShowValue: function () {
              console.log(value);
          }
        };
    }
    

    There's nothing the code calling getModule can do that will change the value in value; value is entirely private to getModule and its contents. It has a copy of the value passed into it (indirectly) from i, it's not a reference to or alias for i.

  • The overwhelming convention in JavaScript is that non-constructor functions start with a lower-case letter, so showValue rather than ShowValue (even when they're methods; think console.log or Math.max).

  • var applies throughout the scope (global scope or a specific function's scope) in which it appears. The two var i declarations in your code are not scoped to the loop, so the second one is unnecessary (but permitted by the language). (ES2015+ have let and const which would be scoped to the loop, but var isn't.)

  • As of ECMAScript fifth edition (2009), JavaScript has a "strict" mode which changes some of the behavior of the language, including what this is in a function that is called without doing anything that would set this. (this is undefined in that situation instead of referring to the global object.) Strict mode also has other benefits, like flagging it up if you accidentally use a variable you haven't declared. To enable it, put "use strict"; at the top of the code. More about strict mode on MDN.

So:

"use strict";
var i;
var arr = [];
function getModule(value){
    return {
      showValue: function () {
          console.log(value);
      }
    };
}
for (i = 0; i < 10; i++){
    arr.push(getModule(i))
}
for (i = 0; i < 10; i++){
    arr[i].showValue();
}
.as-console-wrapper {
  max-height: 100% !important;
}
T.J. Crowder
  • 879,024
  • 165
  • 1,615
  • 1,639
  • 1
    I would also recommend OP to putt `'use strict'` at the top of the code. This would've made the issue with the code more apparent. – Lennholm Nov 07 '17 at 10:16
  • @MikaelLennholm: ...and it's just good advice 99.9999% of the time anyway. :-) Done. – T.J. Crowder Nov 07 '17 at 10:21