JavaScript: multiple instances wrong reference to private property

Question

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

Please post the complete code in the question (you can still [edit] it), not (only) in an external fiddle. — Bergi, Sep 04 '17 at 16:41
Your `frameChanged` and `calculateFrame` functions are [accidentally global](http://blog.niftysnippets.org/2008/03/horror-of-implicit-globals.html) — Bergi, Sep 04 '17 at 16:47

JLRishe · Answer 1 · 2017-09-04T16:48:27.670

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

It was not intentional indeed. Anyway I appreciate your help guys. — Igor Fialko, Sep 04 '17 at 17:20
@IgorFialko Remember to run your code in strict mode. Among other things, it will prevent you from using implicit globals. — JLRishe, Sep 04 '17 at 17:26
I usualy do. But the project from which the snippet is originating has to support IE9 and also has no transpilers, so sometimes these errors appear unfortunately. — Igor Fialko, Sep 04 '17 at 17:33
We plan though to move to babel, typescript etc. to minimize errors of this kind. — Igor Fialko, Sep 04 '17 at 17:35
@IgorFialko Your first comment there makes no sense. Strict mode code can run in IE9 just fine. No transpiler needed. — JLRishe, Sep 05 '17 at 06:27
@JLRishe Hm, support for use strict is firstly introduced in IE11 as far as I understand. https://www.w3schools.com/js/js_strict.asp http://caniuse.com/#feat=use-strict P.S. What I see though after a small research, is that the statement should cause no problems in lower versions. Thanks. — Igor Fialko, Sep 05 '17 at 11:29

JavaScript: multiple instances wrong reference to private property

1 Answers1