JavaScript: multiple instances wrong reference to private property

68 views Asked by At

In the fiddle is a "class" I have written to manage navigation over the data model and a test which shows that multiple instances (starting from second) of this "class" are referencing something wrong.

https://jsfiddle.net/btvmnaxc/ (outputs to console)

Expected output would be

[{"name":"xx"}]
[{"name":"yy"}]

But after setting Elements via setElements, in other methods Elements is empty, strangely only after creating the second instance. I could think that setElements overwrites the reference, but why other methods keep this old reference instead of getting a new one from the var.

Could somebody explain this behavior?

P.S. I probably can think on a solution, as packing vars in a property which is an object.

function Pagination() {
  var props = {Elements:[], ...}
}

P.S.S

function Pagination() {
  var that = this;
  var Elements = [0,1];
  var Frame = [];
  var FrameNumber = 0;
  var EntitiesPerFrame = 25;
  var FrameChangedCB = [];

  this.subscribeFrameChange = function(cb) {
    if (typeof cb === "function") {
      FrameChangedCB.push(cb);
    } else {
      throw new Error("Not a function");
    }
  }

  this.setEntitiesPerFrame = function(entities_per_frame) {
    entities_per_frame = parseInt(entities_per_frame);
    if (entities_per_frame > 0) {
      EntitiesPerFrame = entities_per_frame;
      while (!this.canDisplayFrame(FrameNumber) && FrameNumber > 0) {
        FrameNumber--;
      }
      calculateFrame();
    }
  }

  frameChanged = function() {
    FrameChangedCB.forEach(function(cb) {
      cb();
    });
  }

  this.setElements = function(elements) {
    if (Array.isArray(elements)) {
      Elements = elements;
      calculateFrame();
      console.log("qq");
    } else {
      throw new Error("Can only work with arrays");
    }
  }

  this.getStart = function() {
    return FrameNumber * EntitiesPerFrame;
  }

  this.getEnd = function() {
    var end = (FrameNumber + 1) * EntitiesPerFrame;
    return end > Elements.length ? Elements.length : end;
  }

  this.getEntitiesPerFrame = function() {
    return EntitiesPerFrame;
  }

  calculateFrame = function() {
    var start = that.getStart();
    var end = that.getEnd();
    if (that.canDisplayFrame(FrameNumber)) {
      Frame = Elements.slice(
        start,
        end
      );
      frameChanged();
    } else {
      throw new Error("Boundaries");
    }
  }

  this.canDisplayFrame = function(nr) {
    nr = parseInt(nr);
    var can = false;
    var start = nr * EntitiesPerFrame
    var end = (nr + 1) * EntitiesPerFrame;

    if (start <= Elements.length && nr >= 0) {
      can = true;
    }

    return can;
  }

  this.getFrame = function() {
    return Frame;
  }

  this.next = function() {
    return this.goto(FrameNumber + 1);
  }

  this.prev = function() {
    return this.goto(FrameNumber - 1);
  }

  this.goto = function(frame_nr) {
    var changed = false;
    if (that.canDisplayFrame(frame_nr)) {
      FrameNumber = parseInt(frame_nr);
      calculateFrame();
      changed = true;
    }
    return changed;
  }

  this.getLength = function() {
    return Elements.length;
  }


}


var b = new Pagination();
var a = new Pagination();
a.setElements([{name: 'xx'}]);
b.setElements([{name: 'yy'}]);
console.log(JSON.stringify(a.getFrame()));
console.log(JSON.stringify(b.getFrame()));
1

There are 1 answers

8
JLRishe On

This is happening because you are abusing implicit globals.

Your Pagination function contains two places where a function is assigned to an identifier without using var:

  calculateFrame = function() {
    var start = that.getStart();
    var end = that.getEnd();
    if (that.canDisplayFrame(FrameNumber)) {
      Frame = Elements.slice(
        start,
        end
      );
      frameChanged();
    } else {
      throw new Error("Boundaries");
    }
  }

This will assign this function to a global variable named calculateFrame and any call to calculateFrame() will be calling whichever of those was assigned last (and therefore be using whatever scope it has access to).

To fix this, use var:

  var calculateFrame = function() {
    var start = that.getStart();
    var end = that.getEnd();
    if (that.canDisplayFrame(FrameNumber)) {
      Frame = Elements.slice(
        start,
        end
      );
      frameChanged();
    } else {
      throw new Error("Boundaries");
    }
  }

Or better yet, use a named function declaration:

  function calculateFrame() {
    var start = that.getStart();
    var end = that.getEnd();
    if (that.canDisplayFrame(FrameNumber)) {
      Frame = Elements.slice(
        start,
        end
      );
      frameChanged();
    } else {
      throw new Error("Boundaries");
    }
  }

After fixing the two places where you have this issue, the snippet outputs the expected result.

function Pagination() {
  var that = this;
  var Elements = [0, 1];
  var Frame = [];
  var FrameNumber = 0;
  var EntitiesPerFrame = 25;
  var FrameChangedCB = [];

  this.subscribeFrameChange = function(cb) {
    if (typeof cb === "function") {
      FrameChangedCB.push(cb);
    } else {
      throw new Error("Not a function");
    }
  }

  this.setEntitiesPerFrame = function(entities_per_frame) {
    entities_per_frame = parseInt(entities_per_frame);
    if (entities_per_frame > 0) {
      EntitiesPerFrame = entities_per_frame;
      while (!this.canDisplayFrame(FrameNumber) && FrameNumber > 0) {
        FrameNumber--;
      }
      calculateFrame();
    }
  }

  function frameChanged() {
    FrameChangedCB.forEach(function(cb) {
      cb();
    });
  }

  this.setElements = function(elements) {
    if (Array.isArray(elements)) {
      Elements = elements;
      calculateFrame();
      console.log("qq");
    } else {
      throw new Error("Can only work with arrays");
    }
  }

  this.getStart = function() {
    return FrameNumber * EntitiesPerFrame;
  }

  this.getEnd = function() {
    var end = (FrameNumber + 1) * EntitiesPerFrame;
    return end > Elements.length ? Elements.length : end;
  }

  this.getEntitiesPerFrame = function() {
    return EntitiesPerFrame;
  }

  function calculateFrame() {
    var start = that.getStart();
    var end = that.getEnd();
    if (that.canDisplayFrame(FrameNumber)) {
      Frame = Elements.slice(
        start,
        end
      );
      frameChanged();
    } else {
      throw new Error("Boundaries");
    }
  }

  this.canDisplayFrame = function(nr) {
    nr = parseInt(nr);
    var can = false;
    var start = nr * EntitiesPerFrame
    var end = (nr + 1) * EntitiesPerFrame;

    if (start <= Elements.length && nr >= 0) {
      can = true;
    }

    return can;
  }

  this.getFrame = function() {
    return Frame;
  }

  this.next = function() {
    return this.goto(FrameNumber + 1);
  }

  this.prev = function() {
    return this.goto(FrameNumber - 1);
  }

  this.goto = function(frame_nr) {
    var changed = false;
    if (that.canDisplayFrame(frame_nr)) {
      FrameNumber = parseInt(frame_nr);
      calculateFrame();
      changed = true;
    }
    return changed;
  }

  this.getLength = function() {
    return Elements.length;
  }


}


var b = new Pagination();
var a = new Pagination();
a.setElements([{
  name: 'xx'
}]);
b.setElements([{
  name: 'yy'
}]);
console.log(a.getFrame());
console.log(b.getFrame());