Details on returning an object literal from a closure in JavaScript

Question

Background: I want to rewrite a library (which I did not write) to avoid having the Closure Compiler generate warnings with the advanced option. Per this question JavaScript “this” keyword and Closure Compiler warnings the answer was to rewrite the code using a closure. The intent is to avoid using the keyword this (which generates the compiler warnings).

As the library has a number of functions, I though it would be best for the new closure to return an object literal. I want to understand how this works and any possible ramifications. I therefore wrote the following (meaningless) example as a learning expercise (also here: jsFiddle):

  var CurrencyObject = function(Amount) {
        var money = Amount;
        return {
              "toCents": function() {
                    money *= 100;
                    return money;
              },
              "toDollars": function() {
                    money /= 100;
                    return money;
              },
              "currentValue": money  // currentValue is always value of Amount
        };
  }; // end currencyObject

  var c1 = CurrencyObject(1.99); // what's the difference if the syntax includes `new`?

  alert('cents = ' + c1.toCents() + '\ncurrent money = ' + c1.currentValue + '\ndollars = ' + c1.toDollars() + '\ncurrent money = ' + c1.currentValue);

  var c2 = CurrencyObject(2.99);

  alert('cents = ' + c2.toCents() + '\ncurrent money = ' + c2.currentValue + '\ndollars = ' + c2.toDollars() + '\ncurrent money = ' + c2.currentValue);

  alert('cents = ' + c1.toCents() + '\ncurrent money = ' + c1.currentValue + '\ndollars = ' + c1.makeDollars() + '\ncurrent money = ' + c1.currentValue);

Q1: Why isn't currentValue updated after the call to toCents? (I'm going to guess that it is because currentValue is a literal(?) that it is initialized when CurrencyObject is first executed. If that's the case, what's the syntax for also returning the property currentValue?)

Q2: This syntax (with new) var c1 = new CurrencyObject(1.99); does not change the code's behavior in a way that I can detect, yet I assume there is a difference. What is it?

Q3: When c2 is instantiated, are copies of the functions being created or will c1 and c2 share the same (function) code? (If copies of the functions are being created, what changes do I make to avoid that?)

TIA

BTW: In the event someone is wondering, the symbols in the object literal are quoted to avoid having the Closure-Compiler rename them.

Answer 1

Q1: You've created two variables that contains separate copies of the Amount. One is in the money variable captured in the closure. The other copy is in the currentValue property of your returned object. Those are separate variables that have no connection to one another, other than the initial value of currentValue is initialized with the value of the money variable.

To work-around this issue, you have to settle on only have one place that you store your currentValue and refer to it there. You could only use the money local variable in the closure, but that would require changing the currentValue property to be a getter function (that retrieves the money value) rather than a direct data property.

Or, you could get rid of the money closure variable and only use the currentValue property. That would require you to use this to get that property in the toCents and toDollars methods. In my opinion, the cleaner way to do this would the latter (using this to reference the object's own properties). I have no idea why closure wouldn't want you to do that.

Q2: When you explicitly return an object from a constructor, then it no longer matters whether the constructor is called directly as a function or with the new operator. As you have observed, both create the same result.

Q3: Since your functions are anonymous, each new instantiation will create a new anonymous function object. How efficient or inefficient that is will depend upon the javascript implementation. You could make there only be one function by declaring your functions to be local named functions and referring to that name. Then, you could guarantee that no new function was created.

Answer 2

UPDATE:
I branched your fiddle, and added a couple of methods that all instances will in fact share (ie: no new function objects will be created for each instance): the methods that convert the amount into Euro's or Pounds, for example. I've also omitted the money variable, because it's simply not necessary. To avoid having to use this as much as possible, I've also assigned the returned object to a variable within the closure scope, so that all methods can reference their parent object via that variable name, rather than having to rely on this.
Shared methods, however, will still have to use this occasionally, because they were declared in a "higher" scope, and don't have access to the returnObject variable, simply because it doesn't exist in their scope. Lifting the returnObject variable declaration is no solution in case you were wondering, because then you'll soon find out that you can't create more than 1 instance of the currency object.
Lastly, I've changed the name of the "constructor" to start with a lower case. Technically, your function is not a constructor any longer, and convention is that it therefore can't start with an upper-case letter... Let me know if anything I explained here, or any of my suggested changes are still unclear to you.

The currentValue isn't updated because you changed the money variable's value by doing: money *= 100;. This statement multiplies the money value and assigns it to the same variable. Since this is a primitive, currentValue has its own copy of this value, they're not linked in any way.
Suggestions: use return money/100; to keep the value of money constant. As it now stands calling the toCents method twice is the same as multiplying the original amount by 10,000. To update/set the currentValue on each call to whatever you want it to be, add: this.currentValue = money*100;, which is a little dangerous, or giving your closure access to its own literal by using a named reference (which is safer, but a bit more verbose):

var someClosure = function (amount)
{
    var money = amount;//redundant: you can use amount, it's preserved in this scope, too
    var me = {currentValue:amount,
              toCents: function()
              {
                  me.currentValue = money*100;//<-- use me to refer to the object itself
                  return amount/100;
              }
      };
      return me;
}

There is no reason to use the new keyword: the object that is created by this "constructor" is an object literal, it has only 1 prototype (Object.prototype, and no explicit constructor name). Adding new will make this point to a new object in the function itself, but since you're not using it, and your function returns another object, the object is never returend.

Strictly speaking: some JS engines will create new function objects for each new instance. Some modern objects optimize this, and will in fact only create 1 function object. To be on the safe side, you can wrap another closure around things, to make sure there is only 1 function, regardless of what engine runs your code:

var someObjectGenerator = (function()
{
    var oneOffFunction = function()
    {
        console.log('this function will only be created once');
    }
    return function(amount)
    {
        return {    foo:oneOffFunction,
                    toCents: function()
                    {
                        return amoutn/100;
                    }
                };
    };
};

Of course, functions that are created in a scope above the one that has the money or amount variable won't have access to that variable, so in that case you'll have to create new functions... but JS objects are very cheap, so don't worry about it that much.
Again, most engines deal with this rather well.