current community

your communities

more stack exchange communities

Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite
1

I'm using Node.js and am creating some models for my different objects. This is a simplified version of what they look like at the moment.

var Foo = module.exports = function () {

    var values = { type: 'foo', prop1: '', prop2: '' };

    function model() {}

    model.init = function(val) {
        _.extend(values, val);
        return model;
    }

    model.store = function(cb) {
        db.insert(values.type, values, cb);
    }

    model.prop1 = function(val) {
        if(!arguments.length) return values.prop1;
        values.prop1 = val;
        return model;
    }

    return model;
}

Then I can do:

var foo = Foo();
foo.init({prop1: 'a', prop2: 'b'}).store(function(err) { ... });

I then create another model:

var Bar = module.exports = function () {

    var values = { type: 'foo', prop3: '', prop4: '' };

    function model() {}

    model.init = function(val) {
        _.extend(values, val);
        return model;
    }

    model.store = function(cb) {
        db.insert(values.type, values, cb);
    }

    model.prop3 = function(val) {
        if(!arguments.length) return values.prop3;
        values.prop3 = val;
        return model;
    }

    return model;
}

At which point I realize that a lot of the functions, like model.init and model.store will be reused in each of the models. But while they are exactly the same code, they depend on the local values contained in the model closure.

What's the best way to pull these functions out into a common base class that I can then use to extend all of my models with?

share

locked by Jamal♦ Aug 10 '14 at 0:03

This question exists because it has historical significance, but it is not considered a good, on-topic question for this site, so please do not use it as evidence that you can ask similar questions here. This question and its answers are frozen and cannot be changed. More info: help center.
    
What's the db variable? Is that mongodb? –  Larry Battle Sep 24 '12 at 17:27

1 Answer 1

up vote 4 down vote accepted

Here are some tips:

1) Turn Model into a constructor that accepts an object as the default values.

Note: Constructors should start with a capital letter. Code:

// Model
function Model(defaultValues) {
    this.values = defaultValues || {};
}

Usage:

var modelFoo = new Model({
    type : 'foo',
    prop1 : '',
    prop2 : ''
}); 
console.log( modelFoo.values.type === "foo" );  // true

2) For readability, set the value of module.exports at the end of the file.

Assuming Foo and Bar are in the same file you can export both of them like so.

Code:

module.exports = {
    "Foo" : Foo,
    "Bar" : Bar
};

3) Since Model is a constructor, you can access the interval values by using this and attach functions to it by using prototype.

Also, returning this gives access to the current object instance.

Old Code:

model.init = function (val) {
    _.extend(values, val);
    return model;
}

model.store = function (cb) {
    db.insert(values.type, values, cb);
}

New Code:

Model.prototype.init = function (val) {
    this.values = _.extend(this.values, val);
    return this;
};
Model.prototype.store = function (cb) {
    db.insert(this.values.type, this.values, cb);
    return this;
};

4) Make sure that you define db in the module.

Code:

var db = require("db").db;

5) Create a generic setter and getter function for the properties.

Old Code:

model.prop1 = function (val) {
    if (!arguments.length)
        return values.prop1;
    values.prop1 = val;
    return model;
}

New Code:

Model.prototype.createPropSetterAndGetter = function (propName) {
    this[propName] = (function () {
        return function (val) {
            if (!arguments.length) {
                return this.values[propName];
            }
            this.values[propName] = val;
        };
    }());
};

Usage:

var modelBar = new Model({
    type : 'bar',
    prop3 : '',
    prop4 : ''
});
console.log( typeof modelBar.prop3 === "undefined" ); // true
modelBar.createPropSetterAndGetter("prop3");
console.log( modelBar.prop3() === "" ); // true
modelBar.prop3( "PAY ME!" );
console.log( modelBar.prop3() === "PAY ME!" ); //true and hurry.

Final Result:

var db = require("db").db;

// Model
function Model(defaultValues) {
    this.values = defaultValues || {};
}
Model.prototype.init = function (val) {
    this.values = _.extend(this.values, val);
    return this;
};
Model.prototype.store = function (cb) {
    db.insert(this.values.type, this.values, cb);
    return this;
};
Model.prototype.createPropSetterAndGetter = function (propName) {
    this[propName] = (function () {
        return function (val) {
            if (!arguments.length) {
                return this.values[propName];
            }
            this.values[propName] = val;
        };
    })();
};
// Foo
var Foo = function () {
    var modelFoo = new Model({
            type : 'foo',
            prop1 : '',
            prop2 : ''
        });
    modelFoo.createPropSetterAndGetter("prop1");
    return modelFoo;
};
// Bar
var Bar = function () {
    var modelBar = new Model({
            type : 'bar',
            prop3 : '',
            prop4 : ''
        });
    modelBar.createPropSetterAndGetter("prop3");
    return modelBar;
};

module.exports = {
    "Foo" : Foo,
    "Bar" : Bar
};
share
    
Thanks Larry! I liked the previous structure that I had since it created its own closure which meant that I didn't have to keep calling this all the time so I was hoping that there was a way to extend that. But looks like I'll be heading back to standard prototypes. Thanks again! –  Bill Sep 26 '12 at 0:26

Not the answer you're looking for? Browse other questions tagged or ask your own question.