JavaScript - prototype, getters, setters, functions

Question

I'm just getting to know the whole prototype world, and now I'm trying to do in JS what is normally done in OO languages, namely classes and DAO classes.

I'd be grateful for any comments on whether the code is written in accordance to JS best practices and whatnot. More specifically, I didn't implement getters or setters per se, is my approach still sound?

function Person(obj) {
  this.id = obj.id;
  this.name = obj.name;
  this.surname = obj.surname;
}

Person.prototype.toString = function(){
  return "["+this.id+"] name: " + this.name + ", surname: " + this.surname;
};

/*
Field.prototype = {
    get value(){
        return this._value;
    },
    set value(val){
        this._value = val;
    }
};
*/

function PersonDAO(){
   this.arrayOfPeople = []; 
}

PersonDAO.prototype = {
  save:function(personObj){
    this.arrayOfPeople.push(personObj);
  },
  getAll:function(){
    return this.arrayOfPeople;
  }
};

var John = new Person({
  id: 1,
  name: "John",
  surname: "Smith"
});

var Bob = new Person({
  id: 2,
  name: "Bob",
  surname: "Doe"
});

var pDAO = new PersonDAO();
pDAO.save(John);
pDAO.save(Bob);
console.log(pDAO.getAll()[0].toString());

Answer 1

Though dynamic, you should not force JavaScript it to use complicated patterns that aren't worth the hassle. In your case, the code you want can be simplified into:

var people = [];

people.push({
  id: 1,
  name: "John",
  surname: "Smith"
});

people.push({
  id: 2,
  name: "Bob",
  surname: "Doe"
});

Another thing is that JavaScript has no natural concept of privacy (save closures of course). So when doing some prototypal OOP, almost everything is public. Getters and setters might not hold much purpose.

The key to a big JavaScript app is to never make it big in the first place. Keep components small, simple and easy to debug in case they go wild. Additional baggage will drag you down eventually.

Also, I suggest you don't modify native methods. Though it's common that developers do override some of them, like toString(). But some other code might use it in a way that it expects the native functionality. A general rule in JS is don't modify objects you don't own.

Answer 2

I feel your question is bit abstract, requiring for exactly "point of view" :)

You also did not mention which "normal OO language" you mean. Supposedly Java. And what javascript kind you use - browser or node.js?

Anyway, I personally agree your code is sound enough, though it looks to me that all this piece (itself) do not require using prototype.

function PersonDao(initParams) {
    this.save = function() {
    }
}

This will still work all right.

My humble opinion is that since JavaScript is not normal OO language, you need not pursue all patterns you have in Java for example.

Usually data models do not require to be inherited (though some people have specific view on this issue), only aggregated. Then you need not prototypes for them.

Also note that in most scripting languages DAO is not popular pattern, instead people prefer ActiveRecord here - so that Person will contain methods for save and static method for load (here you may really want address to prototype, perhaps). Though I personally really support DAO-based variant.

Second point, that if you want to move on in java way, you need to have Person properties hidden, not exposed. (I.e. they should be without "this", simply vars inside Person) And by the way you obviously can generate getters for them on the fly, since it is scripting language.

Answer 3

Ok you're definitly headed in the right direction, though there are some things worth mentioning:

function Person(obj)
{
  this.id = obj.id;
  this.name = obj.name;
  this.surname = obj.surname;
}

You're assuming the user will be kind enough to pass an object to this function, that has three properties readily available. If that's the case, why bother with another constructor? You already have an object, no need to add another prototype to the mix.
You're not even checking for the types of these properties. they could be non-existant (undefined), functions, objects, arrays... anything really, even the argument might be undefined. Consider doing:

function Person(o)
{
    o = o || {};//default to object literal
    this.id = o.id || 0;//default id
    this.name = (o.name && o.name == (o.name + '') ? o.name : '') + '';//check type, too
}

Ok, you still have the chance someone using this code omits the new keyword when calling the constructor. How do you deal with that? if the function is being called in the global context, this.id will create a global variable, which isn't what you want at all.
There are several ways of dealing with that, starting with the bad way: catching that error and correcting it:

function Person(o)
{
    if (!this instanceof Person)
    {
        return nuew Person(o);//return new object anyway
    }
    //constructor
}

When a function is called with the new keyword, its call-context will be a new instance of an object, if this is not an instance of Person, it's safe to say the new keyword was omitted. But that's just working around the problem, and leads to BPP (Bad Programming Practice) amongst the users of your code.
Better, but still bad is to silently fail:

if (!this instanceof Person)
{
    return;//returns undefined
}

This is what some classical OO languages do, too: return void, null, nil or whatever. In JS, not being strongly-typed, this won't result in errors until the variable that was assigned undefined is being used as an object. No telling where this'll happen, so debugging might become troublesome after a while.
So, fail loudly it is:

function Person(o)
{
    if (!this instanceof Person)
    {
        throw new Error('Person called as function, is a constructor');
    }
}

That looks alright, doesn't it? Of course not! You don't need to perform that type-check every time, if you simply embrace strict-mode:

function Person(o)
{
    'use strict';
    o = o || {};
    this.id = o.id || 0;
}
Person({id:123});//error!
var p1 = new Person;//OK!

That's it. In strict mode, this doesn't default to the global object anymore, but this is null. In JS, and any other language I know of null.id doesn't add up: you can't get a property of a non-object. Great. That's the constructor sorted. Moving on

Person.prototype.toString = function()
{
  return "["+this.id+"] name: " + this.name + ", surname: " + this.surname;
};

That's OK. You own the Person constructor, so you're free to determine how it's stringified. If ever you come across a situation where this specific toString implementation causes problems, you can easily fix it:

var p1 = new Person;
module.SomFuncThatNeedsDefaultToString(p1);//error because of toString
//FIX:
p1.toString = {}.toString;//assign this instance the default toString method of an object
module.SomFuncThatNeedsDefaultToString(p1);//works fine!
delete p1.toString;//restores the normal Person-behaviour.

Next, the PersonDAO constructor and prototype. The object itself merely looks like a wrapper around a single Array instance, so why?, why not use:

var personDAO = [];
personDAO.save = personDAO.push;//create an alias if you will
personDAO.getAll = function(copy)
{
    copy = !!copy;//coerce to boolean
    if (copy === true)
    {
        return this.slice(0);//shallow copy
    }
    return this;
};
personDAO.toString = (function(nativeToString)
{
    return function()
    {
        for (var i=0;i<this.length;i++)
        {
            this[i] = this[i].toString();//stringify Person instances
        }
        return nativeToString.call(this);
    };
}(personDAO.toString));

This allows you to access, the objects you've stored into the array using either their numeric index, or any custom method you've attached to that particular instance of Array. Go nuts, basically. You could create your own constructor, and keep track of all the properties that were assigned too it, and create a magic length property (as a function in drag, really), and assign new properties using a numeric name (var a = {}; a[1] = 'property'; is valid in js, numbers can be property-names), but such an object was already created for you either use PersonDAO.prototype = []; (and PersonDAO.prototype.constructor = PersonDAO;, but more on that later) to inherit those goodies, or use an array, with its own methods...
Anyway, you're assigning an object literal to the PersonDAO's prototype. There's nothing wrong with that, but you have to be aware of the fact that PersonDAO's instances will behave slightly different, because you've overriden the prototype's constructor property.

function PersonDAO()
{
   this.arrayOfPeople = []; 
}

PersonDAO.prototype = {
    save:function(personObj)
    {
        this.arrayOfPeople.push(personObj);
    },
    getAll:function()
    {
        return this.arrayOfPeople;
    }
};
var dao = new PersonDAO;
if (dao instanceof PersonDAO)
{
    console.log('this will never show up!');
}
var secondDao = new dao.constructor;//secondDao will be a regular object

I'm assuming you've come across code like this:

function Obj1(){}
function Obj2(){}
Obj2.prototype = new Obj1;
Obj2.prototype.constructor = Obj2;//<==!!

By assigning an object literal to the prototype, you're essentially doing the same thing:

Obj2.prototype = new Object();//with a number of properties

If you ever get round to setting up a complex prototype chain, you will break that chain, by not restoring the constructor reference to point back to the actual constructor function. As a result, you can't use an instance to call the constructor if that constructor is, for some reason, not in scope:

//if PersonDAO was not declared globally:
function someF(daoInstance)
{
    var PersonDAO = 'just some var',
    tempDao = new daoInstance.constructor;//doesn't work
    tempDao = new PersonDAO;//error string is not a function.
}

In short, it's no big disaster, but it does brake some (possibly) useful features you normally would have.
Lastly, and this is perhaps more a personal thing: JS takes a lot of abuse, and allows you to mimic classical OO constructions, but it's at its best when using the prototypal model as intended. Couple that with clever/careful use of closures and you'll soon find your code far more performant and a lot more stable:

var personModule = (function()
{
    'use strict';
    var properties = ['id','name','surname'],
    Person = (function(default)
    {
        return function(o)
        {
            o = o || default;
            for (var i=0;i<properties.length;i++)
            {
                this[properties[i]] = o[properties[i]] || default[properties[i]];
            }
        };
    }({id:0,name:'',surname:undefined}));
    Person.prototype.toString = function()
    {
        var i, str = '';
        for (i=0;i<properties.length;i++)
        {
            str += i=== 0 ? '[' + this[properties[i]] + ']' : ' ' + properties[i] + ': ' + this[properties[i]];
        }
        return str;
    };
    return {Person: Person};
}());
//usage:
var johnDoe = new personMode.Person({id: 0, name: 'John', 'surname': 'Doe'});

And do the same thing for the PersonDOA, inside the personModule, and return {Person: Person, PersonDOA: PersonDOA} and you're there. Adding getters and setters is a doddle, too, change the Person constructor, by adding:

this.instanceProperties = properties.slice(0);

To give each instance a copy of the properties array. Then add these two methods to the prototype:

Person.prototype.set = function(name, value)
{
    if (!this.hasOwnProperty(name))
    {
        this.instanceProperties.push(name);
    }
    this[name] = value;
    return this;//chainable
};
Person.prototype.get = function(name)
{
    if (this.hasOwnProperty(name))
    {
        return this[name];
    }
    for (var i = 0;i<this.instanceProperties.length;i++)
    {
        if (this.instanceProperties[i] === name)
        {
            this.instanceProperties.splice(i,1);//remove element from array
            break;
        }
    }
};

Of course, you'll have to change the toString method, too: instead of looping over properties, you'll have to loop over this.instanceProperties, and check for undefined values.

But all this will possibly give you enough food for thought, so good luck with learning JS, it's a great little language that, but Oh so underrated...