Private fields are actually private static fields

[bripkens]

When trying to create a class with private fields as described in the readme, you actually generate a class with private static fields. Pinpointed it to https://github.com/tannhu/jsface/blob/master/jsface.js#L91. When you pass in a function, this function will actually only be executed once. It should be executed for every new instance.

[tnhu]

Hi Ben,

Thanks for the feedback. It is extremely costly to make instance private fields that way. As you point out, the closure needs to be executed for every new instance. By doing that, the whole process of creating a class is happened again. It will impact performance significantly.

Fields are bound to this are actually instance private ones. For example:

var Foo = Class({
  constructor: function(msg) {
    this.msg = msg;                     // instance private field msg
    this.bar = function() {             // instance private field bar
    }
  }
});

[bripkens]

Hi tannhu,

actually these fields are instance public. Sorry for being so picky - I have a situation where I actually need private instance variables and can cope with the object construction overhead (not to actually "hide" private instance properties but to avoid mixin property name clashes).

var Foo = Class({
  constructor: function(msg) {
    this.msg = msg;
    this.bar = function() {
    }
  }
});

var instance = new Foo("I'm not actually private");
console.log(instance.msg);​​​​​

The readme is somewhat misleading because it indicates that jsface supports instance private variables (especially through the distinction between "static-" and "private properties"). It's actually more like public static and private static properties.

Just to give you an example, this is happening in my application:

var Foo = Class({
  constructor: function() {
    this.container = document.body;
  },

  add: function() {
    var newElement = document.createElement('div');
    this.container.appendChild(newElement);     
  }
});

var Bar = Class([Foo], {
  constructor: function() {
    this.container = [];
  }
});

var instance = new Bar();
instance.add(); // Uncaught TypeError: Object  has no method 'appendChild'

The class Bar also uses an instance variable container (as it seems it's an array). Since Foo relies on this variable (see add method), its functionality will be broken.

[tnhu]

Hi Ben,

I think that's not a proper way to implement clases.

You define property container in Foo, which is assigned to document.body in Foo constructor, then you re-assign it to an empty array in Bar constructor (Bar is a sub-class of Foo), then you expect Bar to use the same value that being assigned in Foo.

You either should not re-assign container in Bar or should call Bar's super constructor after re-assigning container.

var Bar = Class(Foo, {
  constructor: function() {
    this.container = [];
    Bar.$super.call(this);
  }
});

[bripkens]

Foo and Bar by themselves would be valid classes. They simple have different interpretations of a container. When mixing them together, unexpected behavior can be observed. Of course, the container property must be private, but since this isn't possible with jsface, I created the example above to illustrate the problem, i.e., a violation of the open/closed principle and name clashes.

I admit that my use case is very specific (in that name clashes happen in the first place) and that support for private properties (or at least automatic name clash avoidance) is not commonly needed. This issue can probably be closed as private properties don't seem to be one of jsface's objectives.