Variable scope incorrectly set-up in Node.JS

Question

The following is a MCVE of my try at BattleShip in NodeJS. Grid.set calls Grid.place which calls Grid.place.methodPlace which tries to call Grid.cells and fails. This is not the way to access such a variable, since this.cells is not in scope. What is the correct way of accessing this variable?

I'm probably making a mess of it. Beginners do that.

"use strict";
function Grid(x, y) {
  var r, c, row;
  this.cells = [];
  this.default = " ";
  for(r = 0 ; r < x ; r++) {
    row = [];
    for(c = 0 ; c < y ; c++) {
      row.push(" ");
    }
    this.cells.push(row);
  }
}

Grid.prototype.place = function(length) {
  function getRandomInt(min, max) {
    return Math.floor(Math.random() * (max - min)) + min;
  };

  var coordinateList = [];
  function methodPlace(indexOfMethodToUse) {
    if (indexOfMethodToUse == 0) {
      var rndX = getRandomInt(0,(9-length));
      var rndY = getRandomInt(0,9)
      for(var i = 0 ; i <= rndX + length ; i++) {
        if (this.cells[rndX+i][rndY] == this.default) {   // <=====
          coordinateList.push(rndX+i,rndY);
        }
      };
    }
    console.log(coordinateList);
    return coordinateList;
  }
  methodPlace(0);
};

Grid.prototype.set = function(ac) {
  for(var i = 0 ; i <= ac ; i++) {
    this.place(2);
  }
}

var friendlyGrid = new Grid(10,10);
friendlyGrid.set(1,2,1,1);

You could make methodPlace a normal method, so it can access this like place() and set() can. If it's private on purpose, you need this way and you'll have to do self = this or something to make this accessible in your private method. Or you can manually call methodPlace with this: methodPlace.call(this, 0) — Rudie
– Rudie, Commented May 30, 2015 at 12:58
@Rudie It's private on purpose for so far that I thought this would keep the structure clean. This is the method using the function so there's no reason to put it on a higher layer, right? — Mast
– Mast, Commented May 30, 2015 at 13:08
No reason, but probably cleaner, more readable, better debuggable, simpler syntax etc. No need for .call() and/or self because it's a normal method. If it's not private for a good reason, I'd just make it public. The .call() solution is pretty readable and short too though. — Rudie
– Rudie, Commented May 30, 2015 at 14:23

Rudie · Accepted Answer · 2015-05-30 14:29:05Z

At least 2 solutions:

Make the method public, like set() and place() If you do that, it's callable with this.methodPlace(0) and it would know this.
Use .call() to inject context If you do that, it's callable with methodPlace.call(this, 0) and it would know this.

If there's no good reason to have the method be private, I'd make it public: more readable, cleaner, better debuggable, simpler syntax etc. If there is a good reason for it to be private (access), I'd use .call()

There's another solution:

Copy this to self and use that internally I don't like this, because there'll be self and this floating around, but you could copy the class/object this to self and use self instead of this in the private method (where this has changed):
(this is panta82's solution, but using self instead of that, which illustrates my dislike)

.

var coordinateList = [];
var self = this;
function methodPlace(indexOfMethodToUse) {
  // ... //
      if (self.cells[rndX+i][rndY] == self.default) {   // <=====

panta82 · Accepted Answer · 2015-05-30 14:24:17Z

0

Grid.prototype.place = function(length) {
  var that = this;
  // ... the rest of the code

Then later, use "that" instead of "this".

answered May 30, 2015 at 14:24

panta82

2,7213 gold badges22 silver badges38 bronze badges

Collectives™ on Stack Overflow

Variable scope incorrectly set-up in Node.JS

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related