sylvainpolletvillard / ObjectModel

Strong Dynamically Typed Object Modeling for JavaScript
http://objectmodel.js.org
MIT License
467 stars 28 forks source link

Constructors and Inheritance for Models #10

Closed johncrenshaw closed 8 years ago

johncrenshaw commented 8 years ago

Trying to build an Active Record like layer on top of ObjectModel, but it isn't working out so well. This is the first of several issues I have run into. I'm circumventing them in various ways, but ideally the core would be better suited to these sorts of behaviors.

The constructors used by ObjectModel are highly irregular, and there doesn't seem to be a good way to do basic inheitance using common patterns. Using Model.extend() also gives goofy results. I finally had to fall back to using Model as just a validation utility.

Basically the problem is that traditional patterns don't work for the following (also couldn't get Model.extend() to give working results):

var ActiveRecord = Model({});

// How do I set a constructor for ActiveRecord?

var People = // How do I properly inherit ActiveRecord, and also extend the constructor

sylvainpolletvillard commented 8 years ago

Hi john,

At a time, I was thinking of adding a constructor method to object models, which let you reproduce the common patterns you are referring to (like constructor inheritance, aka super() ).

The problem is that object models are meant to provide type validation by overriding getters and setters of every property of the object. If the lib needs to wait for the constructor to return a full, ready-to-use object, that means we can not provide type validation inside the constructor code. I think it breaks the design of the library if we let users have non-type-safe code inside models declarations.

That's why I recommend using factory functions to construct objets, outside of model declarations. See "How do I declare a constructor function to be called on instanciation before validating my model ?" here : http://objectmodel.js.org/#common-questions

I understand that it may be inconsistent with some traditional patterns, and I am open to discussion.

johncrenshaw commented 8 years ago

The biggest problem with this solution is that the prototype doesn't work. Imagine a User.prototype.getDisplayName() method. Because your solution discards "this" and returns something else, the entire User prototype is ignored. We could do some manual prototype cloning or set up the prototype in a goofy way, but that just kicks the can down the road again, and classes that inherit User have the same problem.

It seems to me that a relatively minor change to getProxy (to define the properties on this, rather than on a new proxy object) would go a long way to resolving this. From there you could add an init(properties), and make properties optional on the constructor (pass through to init()). Then the following common inheritance pattern becomes available:

        var UserModel = Model.Object({
            firstName: String,
            lastName: String
        });

        var User = function(properties){ // properties is optional
            // Object creation logic goes here.
            UserModel.call(this, properties); // calls this.init() if properties is defined
        };
        User.prototype = Object.create(UserModel.prototype);
        User.prototype.constructor = User;

        User.prototype.init = function (properties) {
            // Initialization of object values goes here
            return UserModel.prototype.init.call(this, properties);
        };

        User.prototype.getDisplayName = function () {
            return properties.firstName + " " + properties.lastName;
        };

In terms of code logic, this is not substantially different from your proposed constructor, and is in fact slightly better because it avoids unnecessary denormalization of firstName and lastName into a fullName property, instead providing on demand access to a computed full name through getDisplayName. This is compatible with Object.create, inherits the prototype, and can be inherited from. As nearly as I understand this should work with ECMAScript 6 classes as well, since that is just syntactic sugar on top of this inheritance paradigm.

Your concern with type safety of object values can be resolved easily by throwing in the constructor with a meaningful error if any of the model values has already been set to an invalid value (you can't throw just any time it is set, because you are storing defaults on the prototype, which means they will be set already if you have defaults). This is in fact nearly identical to the current construction behavior, because validate() currently throws, and that is the only type validation that defaults get at the moment. The only difference would be the allowance for a typesafe model that does not yet have all property values set (which is probably actually a good thing, because it opens the door for procedural construction of the object, which is sometimes necessary for complex objects in the real world.)

sylvainpolletvillard commented 8 years ago

Model instances inherit from their model's prototype. Default values are properties set on the model's prototype, so it uses prototypal inheritance when not set.

I would write your example this way:

var User = Model.Object({
    firstName: String,
    lastName: String,
    getDisplayName: Model.Function().return(String)
});

User.init = function (properties) {
    // Initialization of object values goes here
    properties.lastName = "Dalton";
    return new User(properties);
};

User.prototype.getDisplayName = function () {
    return this.firstName + " " + this.lastName;
};

var joe = User.init({ firstName: "joe" });
var jack = User.init({ firstName: "jack" });

console.log(joe.getDisplayName(), jack.getDisplayName());
console.log(joe instanceof User);

All the setConstructor, ensureProto stuff you see in code is here to make sure the prototypal chain work as intended, despite the fact we are using proxy objects. Returning a proxy object is the main mechanism behind ObjectModel. getProxy is actually the first piece of code I write for this library.

The main reason to use a proxy object instead of directly overriding the object properties is to be able to smoothly move to ES6 Proxies. Proxies are necessary to provide stronger validation (like property descriptors), and eliminate all the caveats of getter/setters. Above all, Proxies let you validate dynamically added properties, which is a must-have for validating Arrays and objects without all their properties set in the first place. This is impossible to do with getters/setters without using a custom API like $set (Vue.js) or ugly stuff like dirty checking (Angular). Some inner optimizations for Array models already use ES6 proxies when available, and I plan to use them more now they are well supported on major browsers.

There are some other technical reasons. Overriding properties with getter/setters implies storing the original values in a backup object, which must be an exact clone of the original object. This is quite challenging and can cause numerous side effects, especially if the object already has custom property accessors. Using a proxy wrapper and not touching the original object is generally safer. Finally, some features like null-safe object traversal also require using a proxy.

I hope that explains it a little better. Is there any particular reason why you can not use a factory function for your object creation logic ?

sylvainpolletvillard commented 8 years ago

I think this issue can be considered resolved. In short: use factory functions if you need a specific behaviour at object creation.