Module pattern and this

Question

I am using the module pattern for my JavaScript "classes". Is there any significant downside to declaring a var self outisde of the class I am returning and then setting it to this inside the class constructor so that I don't have to worry about the context switching when I don't want it to. In this small example it's probably unnecessary, this is just an example.

Example:

    var Seat = (function() {
      var self = null;
      function Seat(startX, startY, inputSeatNumber, inputTableNumber) {
        self = this;
        self.radius = 10;
        self.x = startX; self.y = startY;
        self.seatNumber = inputSeatNumber;
        self.tableNumber = inputTableNumber;
      }

      Seat.prototype.moveTo = function(newX, newY) {
        if(newX >= 0 && newY >= 0) {
          self.x = newX; self.y = newY;
        }
      };

      return Seat;
    })();

EDIT: example added

var SeatingChartView = (function() {
  function SeatingChartView(canvas_id, seatingChartController, seatForm) {
    this.stage = new createjs.Stage(canvas_id);
    this.controller = seatingChartController;
    this.seatForm = seatForm;

    this.disableRightClick(canvas_id);
  }

  SeatingChartView.prototype.render = function() {
    this.stage.update();
  }

  SeatingChartView.prototype.addSeat = function(newSeat) {
    var newCircle = new createjs.Shape();
    newCircle.graphics.beginFill("black").drawCircle(0, 0, 10);
    newCircle.x = newSeat.x;
    newCircle.y = newSeat.y;
    newCircle.seat = newSeat;
    newCircle.on('click', function(event) {
      if(event.nativeEvent.button == 2) {
        this.seatForm.open(event.currentTarget.seat);
      }
    });
    newCircle.on('pressmove', this.controller.moveSeat)
    this.stage.addChild(newCircle);
  }

  SeatingChartView.prototype.removeSeat = function(seat) {
    this.stage.children.forEach(function(child) {
      if(child.seat === seat) {
        this.stage.removeChild(child);
      }
    });
  }

  SeatingChartView.prototype.setBackground = function(imageLocation) {
    this.background = new createjs.Bitmap(imageLocation);
    window.setTimeout(function() {
      this.stage.canvas.width = this.background.image.width;
      this.stage.canvas.height = this.background.image.height;
      this.stage.addChild(this.background);
      this.stage.update();
    }.bind(this), 500);
  }

  SeatingChartView.prototype.disableRightClick = function(canvas_id) {
    $(function() {
      $('#' + canvas_id).bind('contextmenu', function(e) {
        return false;
      });
    });
  }

return SeatingChartView;
})();

Answer 1

This would totally negate the purpose of "classing". But in JS it's called prototyping.

Principally you want the base prototype to be "copied" when creating new instances. The base prototype should be shielded from changes when extended.

Suppose you have done what you did, all instances of Seat will have the same properties. Even worst, when creating new "copies" of Seat, all other previously created copies will have their values changed.

Since you want this to maintain reference to Seat, I would recommend using the following pattern:

var Base = {
    init: function(arg) {
        this.name = arg;
    },
    getName: function() {
        return this.name;
    }
}
Base.init('foo');
Base.getName(); // returns 'foo'

Your transformed code:

var Seat = {
    init: function(startX, startY, inputSeatNumber, inputTableNumber) {
        this.radius = 10;
        this.x = startX; 
        this.y = startY;
        this.seatNumber = inputSeatNumber;
        this.tableNumber = inputTableNumber;
    },
    moveTo: function(newX, newY) {
        if (newX >= 0 && newY >= 0) {
            this.x = newX; this.y = newY;
        }
    },
    setBackground: function(imageLocation) {
        var self = this;
        this.background = new createjs.Bitmap(imageLocation);
        setTimeout(function() {
            self.stage.canvas.width = self.background.image.width;
            self.stage.canvas.height = self.background.image.height;
            self.stage.addChild(self.background);
            self.stage.update();
        }, 500);
    }
}

Extend the prototype:

var vipSeat = Object.create(Seat);
vipSeat.init( //your init values )

You can also not create an init method and simply use Object.create's second argument to assignment initial values to the prototype: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/create#Example:_Using_propertiesObject_argument_with_Object.create

Answer 2

Yes, by doing it this way, all instances of Seat will have the same this, causing problems all over the place. Just remove the var self and use this in all places where you were using self. In the code you've given, there's no point where you will lose reference to this.

(@added example) Now your question makes more sense.

Instead of trying to handle this for all methods at once, you'll have to handle it at each point where you're using a function that has a different this (any function that isn't on the prototype or instance).

If you don't need this inside the callback, I would just use .bind to make the instance this available inside. Note however that .bind isn't supported in some (very)old versions of IE, so you'll either need a polyfil to work for those, or store this in a var.

  SeatingChartView.prototype.addSeat = function(newSeat) {
    var newCircle = new createjs.Shape();
    newCircle.graphics.beginFill("black").drawCircle(0, 0, 10);
    newCircle.x = newSeat.x;
    newCircle.y = newSeat.y;
    newCircle.seat = newSeat;
    newCircle.on('click', function(event) {
      if(event.nativeEvent.button == 2) {
        this.seatForm.open(event.currentTarget.seat);
      }
    }.bind(this)); // modified here, added `.bind(this)`
    newCircle.on('pressmove', this.controller.moveSeat)
    this.stage.addChild(newCircle);
  }

Answer 3

A more practical demo example might be something like this, where you want to make sure this is the instance of the class.

function Foo() {
    var _this = this;

    _this.someItem = {};

    _this.go = function() {
        doSomethingElse(function(result) {
            _this.someItem.something = result; // _this and this are different
        });
    };
};

function doSomethingElse(callback) {
    callback('asdf');
}

var foo = new Foo();
foo.go();

For your example using that pattern, you can define the _this in each method if it would be any benefit (this one wouldn't, but a more complex example might):

Seat.prototype.moveTo = function(newX, newY) {
    var _this = this;
    if(newX >= 0 && newY >= 0) {
        _this.x = newX; _this.y = newY;
    }
};

Answer 4

In that case every new instance of Seat will share the newest Self object since it is set in the constructor. You should avoid doing this.