Mast
Mast

Reputation: 1884

Variable scope incorrectly set-up in Node.JS

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);

Upvotes: 0

Views: 30

Answers (2)

Rudie
Rudie

Reputation: 53781

At least 2 solutions:

  1. Make the method public, like set() and place() If you do that, it's callable with this.methodPlace(0) and it would know this.
  2. 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:

  1. 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) {   // <=====

Upvotes: 2

panta82
panta82

Reputation: 2721

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

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

Upvotes: 0

Related Questions