1

I am relatively new to JavaScript, and am having trouble grasping why this error occurs:

TypeError: Attempted to assign to readonly property. MyTimer.js: 35

I understand that this error is displayed because I am using strict mode, but I enabled strict mode to help me debug this Object.

The call to create the MyTimer singleton is:

var simTimer = new SimTimer();

Then I add a task to be executed in MyTimer as follows:

var task = function(){
    console.log("performing task.");
};

simTimer.addTask(task);

Lastly, this is the MyTimer Object (line 35 is marked):

var MyTimer = (function () {
    "use strict";

    // var timer;
    var tasks;

    /**
     * If an instance of MyTimer exists, it will be saved to this variable and returned
     * by any subsequent calls to this constructor, thus only one instance (stored in this
     * variable) will be accessible.
     * @private
     */
    var instance;

    /**
     * Called to initialize this MyTimer Object, or return #instance if it already contains
     * an instance of this Object.
     */
    function Singleton() {
        if (instance) {
            return instance;
        }
        instance = this;
        tasks = $.Callbacks();
        this.timer = setInterval(function()
        {
            this.tasks.fire();
        }, 1000);//<---- THIS IS LINE 35!

        this.addTask = function(task)
        {
            this.tasks.add(task);
        };

        this.removeTask = function(task)
        {
            this.tasks.remove(task);
        };
    }

    //instance accessor
    Singleton.getInstance = function () {
        return instance || new Singleton();
    };

    return Singleton();

}());

What have I failed to grasp? I have read through a lot of documentation on Module Patterns, and have successfully written Singletons before - so where do I go wrong here?

** EDIT: **

I was able to get the correct behavior by removing var tasks, and creating it within Singleton using this. The working version of the function now looks like this:

function Singleton() {
    if (instance) {
        return instance;
    }
    instance = this;

    this.tasks = $.Callbacks();

    this.timer = setInterval(function(){
        instance.tasks.fire();
    }, 1000);
    this.removeTask = function(task)
    {
        instance.tasks.remove(task);
    };
    this.addTask = function(task)
    {
        instance.tasks.add(task);
    };

}

So I still don't fully understand - why did this change fix it? Was it a scope issue after all?

Phil
  • 35,852
  • 23
  • 123
  • 164

3 Answers3

1

If I am reading your code right, you have a scope issue here

this.timer = setInterval(function()
{
    this.tasks.fire();  <-- this will be in window scope
}, 1000);

It should be

this.timer = setInterval(function()
{
    instance.tasks.fire(); 
}, 1000);
epascarello
  • 204,599
  • 20
  • 195
  • 236
  • +1. This is definitely a scope issue, and the tip you gave did help - but it wasn't the source of the issue I was working on. Would you know why removing the line `var tasks` from outside of `Singleton()` solved my issue? – Phil Mar 29 '13 at 21:06
  • because it becomes a global variable and this is window so it is window.tasks. – epascarello Mar 30 '13 at 01:13
1

I believe the following strict mode restriction is the explanation.

If this is evaluated within strict mode code, then the this value is not coerced to an object. A this value of null or undefined is not converted to the global object and primitive values are not converted to wrapper objects. The this value passed via a function call (including calls made using Function.prototype.apply and Function.prototype.call) do not coerce the passed this value to an object (10.4.3, 11.1.1, 15.3.4.3, 15.3.4.4).

When you call

return Singleton();

The this value will actually be undefined. Try doing this.

return {
    getInstance: function () {
        return instance || new Singleton();
    }
};
ChaosPandion
  • 77,506
  • 18
  • 119
  • 157
0

Not sure if this is the cause, but looks like the line that reads "tasks = $.Callbacks();" should be "this.tasks = $.Callbacks();". Also, as you are providing an instance as a callback, you will loose the 'this' binding. You function bodys that call anything with "this." should instead use a var that captures this in the outer closure (which looks like 'instance' does.

So for example, the method in question would read:

this.timer = setInterval(function() { instance.tasks.fire(); }, 1000);

Bill Ticehurst
  • 1,728
  • 12
  • 14