What's wrong with this style of coding JavaScript? (closures vs. prototypes)

Question

We have been debating how best to handle objects in our JS app, studying Stoyan Stefanov's book, reading endless SO posts on 'new', 'this', 'prototype', closures etc. (The fact that there are so many, and they have so many competing theories, suggests there is no completely obvious answer).

So let's assume the we don't care about private data. We are content to trust users and developers not to mess around in objects outside the ways we define.

Given this, what (other than it seeming to defy decades of OO style and history) would be wrong with this technique?

// namespace to isolate all PERSON's logic
var PERSON = {};

// return an object which should only ever contain data.
// The Catch: it's 100% public

PERSON.constructor = function (name) {
     return {
         name: name
     } 
}

// methods that operate on a Person
// the thing we're operating on gets passed in

PERSON.sayHello = function (person) {
     alert (person.name); 
}

var p = PERSON.constructor ("Fred");
var q = PERSON.constructor ("Me");

// normally this coded like 'p.sayHello()'
PERSON.sayHello(p);
PERSON.sayHello(q);

Obviously:

1. There would be nothing to stop someone from mutating 'p' in unholy ways, or simply the logic of PERSON ending up spread all over the place. (That is true with the canonical 'new' technique as well).
2. It would be a minor hassle to pass 'p' in to every function that you wanted to use it.
3. This is a weird approach.

But are those good enough reasons to dismiss it? On the positive side:

1. It is efficient, as (arguably) opposed to closures with repetitive function declaration.
2. It seems very simple and understandable, as opposed to fiddling with 'this' everywhere.

The key point is the foregoing of privacy. I know I will get slammed for this, but, looking for any feedback. Cheers.

Answer 1

There's nothing inherently wrong with it. But it does forgo many advantages inherent in using Javascript's prototype system.

• Your object does not know anything about itself other than that it is an object literal. So instanceof will not help you to identify its origin. You'll be stuck using only duck typing.
• Your methods are essentially namespaced static functions, where you have to repeat yourself by passing in the object as the first argument. By having a prototyped object, you can take advantage of dynamic dispatch, so that p.sayHello() can do different things for PERSON or ANIMAL depending on the type Javascript knows about. This is a form of polymorphism. Your approach requires you to name (and possibly make a mistake about) the type each time you call a method.
• You don't actually need a constructor function, since functions are already objects. Your PERSON variable may as well be the constructor function.

What you've done here is create a module pattern (like a namespace).

Here is another pattern that keeps what you have but supplies the above advantages:

function Person(name)
{
    var p = Object.create(Person.prototype);
    p.name = name; // or other means of initialization, use of overloaded arguments, etc.
    return p;
}
Person.prototype.sayHello = function () { alert (this.name); }

var p = Person("Fred"); // you can omit "new"
var q = Person("Me");
p.sayHello();
q.sayHello();
console.log(p instanceof Person); // true
var people = ["Bob", "Will", "Mary", "Alandra"].map(Person);
        // people contains array of Person objects

Answer 2

Yeah, I'm not really understanding why you're trying to dodge the constructor approach or why they even felt a need to layer syntactical sugar over function constructors (Object.create and soon classes) when constructors by themselves are an elegant, flexible, and perfectly reasonable approach to OOP no matter how many lame reasons are given by people like Crockford for not liking them (because people forget to use the new keyword - seriously?). JS is heavily function-driven and its OOP mechanics are no different. It's better to embrace this than hide from it, IMO.

First of all, your points listed under "Obviously"

1. Hardly even worth mentioning in JavaScript. High degrees of mutability is by-design. We're not afraid of ourselves or other developers in JavaScript. The private vs. public paradigm isn't useful because it protects us from stupidity but rather because it makes it easier to understand the intention behind the other dev's code.

2. The effort in invoking isn't the problem. The hassle comes later when it's unclear why you've done what you've done there. I don't really see what you're trying to achieve that the core language approaches don't do better for you.

3. This is JavaScript. It's been weird to all but JS devs for years now. Don't sweat that if you find a better way to do something that works better at solving a problem in a given domain than a more typical solution might. Just make sure you understand the point of the more typical approach before trying to replace it as so many have when coming to JS from other language paradigms. It's easy to do trivial stuff with JS but once you're at the point where you want to get more OOP-driven learn everything you can about how the core language stuff works so you can apply a bit more skepticism to popular opinions out there spread by people who make a side-living making JavaScript out to be scarier and more riddled with deadly booby traps than it really is.

Now your points under "positive side,"

1. First of all, repetitive function definition was really only something to worry about in heavy looping scenario. If you were regularly producing objects in large enough quantity fast enough for the non-prototyped public method definitions to be a perf problem, you'd probably be running into memory usage issues with non-trivial objects in short order regardless. I speak in the past tense, however, because it's no longer really a relevant issue either way. In modern browsers, functions defined inside other functions are actually typically performance enhancing due to the way modern JIT compilers work. Regardless of what browsers you support, a few funcs defined per object is a non-issue unless you're expecting tens of thousands of objects.

2. On the question of simple and understandable, it's not to me because I don't see what win you've garnered here. Now instead of having one object to use, I have to use both the object and it's pseudo-constructor together which if I weren't looking at the definition would imply to me the function that you use with a 'new' keyword to build objects. If I were new to your codebase I'd be wasting a lot of time trying to figure out why you did it this way to avoid breaking some other concern I didn't understand.

My questions would be:

Why not just add all the methods in the object literal in the constructor in the first place? There's no performance issue there and there never really has been so the only other possible win is that you want to be able to add new methods to person after you've created new objects with it, but that's what we use prototype for on proper constructors (prototype methods btw are great for memory in older browsers because they are only defined once).

And if you have to keep passing the object in for the methods to know what the properties are, why do you even want objects? Why not just functions that expect simple data structure-type objects with certain properties? It's not really OOP anymore.

But my main point of criticism

You're missing the main point of OOP which is something JavaScript does a better job of not hiding from people than most languages. Consider the following:

function Person(name){
    //var name = name; //<--this might be more clear but it would be redundant
    this.identifySelf = function(){ alert(name); }
}

var bob = new Person();
bob.identifySelf();

Now, change the name bob identifies with, without overwriting the object or the method, which are both things you'd only do if it were clear you didn't want to work with the object as originally designed and constructed. You of course can't. That makes it crystal clear to anybody who sees this definition that the name is effectively a constant in this case. In a more complex constructor it would establish that the only thing allowed to alter or modify name is the instance itself unless the user added a non-validating setter method which would be silly because that would basically (looking at you Java Enterprise Beans) MURDER THE CENTRAL PURPOSE OF OOP.

Clear Division of Responsibility is the Key

Forget the key words they put in every book for a second and think about what the whole point is. Before OOP, everything was just a pile of functions and data structures all those functions acted on. With OOP you mostly have a set of methods bundled with a set of data that only the object itself actually ever changes.

So let's say something's gone wrong with output:

• In our strictly procedural pile of functions there's no real limit to the number of hands that could have messed up that data. We might have good error-handling but one function could branch in such a way that the original culprit is hard to track down.

• In a proper OOP design where data is typically behind an object gatekeeper I know that only one object can actually make the changes responsible.

Objects exposing all of their data most of the time is really only marginally better than the old procedural approach. All that really does is give you a name to categorize loosely related methods with.

Much Ado About 'this'

I've never understood the undue attention assigned to the 'this' keyword being messy and confusing. It's really not that big of a deal. 'this' identifies the instance you're working with. That's it. If the method isn't called as a property it's not going to know what instance to look for so it defaults to the global object. That was dumb (undefined would have been better), but it not working properly in that scenario should be expected in a language where functions are also portable like data and can be attached to other objects very easily. Use 'this' in a function when:

• It's defined and called as a property of an instance.

• It's passed as an event handler (which will call it as a member of the thing being listened to).

• You're using call or apply methods to call it as a property of some other object temporarily without assigning it as such.

But remember, it's the calling that really matters. Assigning a public method to some var and calling from that var will do the global thing or throw an error in strict mode. Without being referenced as object properties, functions only really care about the scope they were defined in (their closures) and what args you pass them.