Sometimes I think I'm OK at JavaScript (well: I guess I'm OK at it [he says, whilst seesawing his hand]), and then sometimes I put this to the test and conclude I'm actually a bit shit. In this episode, I cover the journey from former to latter.
Important note:
Given the nature of this article, most of the code in it is wrong, and I am in no-way advocating its usage. The raison d'être of the article is demonstrating bad / wrong / pointless code. Do not copy this code. Do not think the code in here is a solution to any problem you might have.We've been trying to get coherence in our approach to JavaScript for years. But it always just seems to be a series of false starts, for one reason or another. But I still chip away at trying to improve things where I can. This tends to involve a lot of experimentation, and not much ground made.
A number of years ago we had my mate Chris Kobrzak on the team, and he'd migrated from being a CFML developer to being a dedicated JavaScript developer. This was excellent as he say to it we had a formal JavaScript coding standard, and actively mentored it. During his tenure I think all of us improved our JS no end. However his tenure was short, and he moved onto other challenges after a while. For one reason or another the role was never re-filled (although we needed it more than ever), and our progress on that improvement has stalled. I think we're all still a bunch better than we were then, but this is more down to repetition and attrition more than anything else.
One legacy that Chris left us was a formalised approach to defining JavaScript "classes", wherein we unified on the standard "prototype-based" approach. This won't seem like news, but we had several different variations of inline-objects / kinda-classes / wodges-of-procedural-code going at once. Basically each dev started a new file however they felt like at the time. Or sometimes in the middle of a file, change tactic / style.
The basic rules of the coding standard were:
- acknowledge that JavaScript is OO but not in the sense we were used to (coming from CFML), and don't bother trying to pretend it's traditional OO by trying to do inheritance or non-supported stuff like that.
- Namespace everything.
- We emulated classes using the prototypical function approach, with a consistency rule that the there was only one class per file, the file was named the same as the class, and the namespace of the class was reflected in the directory hierarchy.
- Separate the definition of the class from the usage of the class, ie: in different files, indeed different directory trees.
- Object methods were always added to the main function's prototype.
- "Class methods" were added to the function directly.
var ns = ns || {};
ns.Person = function(firstName, lastName, dob, status){
this.firstName = firstName;
this.lastName = lastName;
this.dob = dob;
this.status = status;
};
ns.Person.capitaliseName = function(name){
return name.replace(/(^|\b)([a-z])/g, function(match){return match.toUpperCase();});
}
ns.Person.prototype.getFullName = function(){
return ns.Person.capitaliseName(this.firstName)
+ " "
+ ns.Person.capitaliseName(this.lastName);
};
ns.Person.prototype.getAgeInYears = function () {
return new Date().getFullYear() - this.dob.getFullYear();
};
ns.Person.prototype.setStatus = function (status) {
this.status = status;
};
ns.Person.prototype.getStatus = function () {
return this.status;
};
ns.Person.prototype.toJSON = function(){
return {
firstName : this.firstName,
lastName : this.lastName,
fullName : this.fullName,
dob : this.dob,
status : this.status
};
};
Don't worry too much about the code, but here we have a class for a Person which has properties firstName, lastName, date-of-birth, and status. We have object methods to get the person's full name; their age in years; plus set and get a status; and also a method for
JSON.stringify() to use when serialising the object. The class also exposes a class method capitaliseName() which can be used for out-of-object situations where one just needs to capitalise a name, but doesn't need an object. The whole thing is namespaced (unimaginatively "ns" in this case) to keep our application's stuff away from other JavaScript stuff.This file is called Person.js and is at something like /webapp/public/js/lib/ns/Person.js.
It could be used like this:
var person = new ns.Person("kiri", "te kanawa", new Date("1944-03-06"));
console.dir(person);
console.log(person.firstName);
console.log(person.lastName);
console.log(person.getFullName());
console.log(JSON.stringify(person));
console.log(ns.Person.capitaliseName("jean batten"));
var secondPerson = new ns.Person("georgina", "beyer", new Date("1957-11-01"));
console.dir(secondPerson);
console.log(secondPerson.firstName);
console.log(secondPerson.lastName);
console.log(secondPerson.getFullName());
secondPerson.setStatus("Active");
console.log(secondPerson.getStatus());
console.log(JSON.stringify(secondPerson));
Now I hasten to add that all the JavaScript code we're talking about here is client-side JavaScript, I perhaps shoulda mentioned that before. That said, I'm running it via node as the output is easier to copy/paste here:
{ firstName: 'kiri',
lastName: 'te kanawa',
dob: Mon Mar 06 1944 00:00:00 GMT+0000 (GMT Standard Time),
status: undefined }
kiri
te kanawa
Kiri Te Kanawa
{"firstName":"kiri","lastName":"te kanawa","dob":"1944-03-06T00:00:00.000Z"}
Jean Batten
{ firstName: 'georgina',
lastName: 'beyer',
dob: Fri Nov 01 1957 00:00:00 GMT+0000 (GMT Standard Time),
status: undefined }
georgina
beyer
Georgina Beyer
Active
{"firstName":"georgina","lastName":"beyer","dob":"1957-11-01T00:00:00.000Z","status":"Active"}
C:\src>
lastName: 'te kanawa',
dob: Mon Mar 06 1944 00:00:00 GMT+0000 (GMT Standard Time),
status: undefined }
kiri
te kanawa
Kiri Te Kanawa
{"firstName":"kiri","lastName":"te kanawa","dob":"1944-03-06T00:00:00.000Z"}
Jean Batten
{ firstName: 'georgina',
lastName: 'beyer',
dob: Fri Nov 01 1957 00:00:00 GMT+0000 (GMT Standard Time),
status: undefined }
georgina
beyer
Georgina Beyer
Active
{"firstName":"georgina","lastName":"beyer","dob":"1957-11-01T00:00:00.000Z","status":"Active"}
C:\src>
OK, so that demonstrates everything works A-OK, but also demonstrates something that tends to irk me with JavaScript: for the object methods to access the state of the object, the state needs to be public (accessed via the object's this reference). There's no sense of privacy to the object's properties, and it means all (shared) methods are also really public. In reality this is not often an issue, other than the fact sometimes we find ill-thought-out code battering into a property directly instead of using the class's API. This is a non-theoretical issue: it has caused us problems in the past. Mostly with refactoring. It's always best to keep one's API "on point", as it reduces refactoring challenges.
And it just irks me. I also know I am not alone in this irkery.
One thing some of the bods at work have been looking at recently is YUI's Module Pattern, which kinda deals with the property privacy thing by leveraging closure. Outwardly this sounds all right actually, and when I only fleetingly looked at some example code it looked like the business for us. Here's a reimplementation of the above class using the traditional Module Pattern.
var ns = ns || {};
ns.person = (function(firstName, lastName, dob){
var fullName = firstName + " " + lastName;
var private = {
status : undefined
};
var capitalise = function(name){
return name.replace(/(^|\b)([a-z])/g, function(match){return match.toUpperCase();});
};
return {
getFullName : function(){
return capitalise(fullName);
},
capitaliseName : capitalise,
getAgeInYears : function () {
return new Date().getFullYear() - private.dob.getFullYear();
},
setStatus : function (status) {
private.status = status;
},
getStatus : function(){
return private.status;
},
toJSON : function() {
return {
firstName : firstName,
lastName : lastName,
fullName : fullName,
dob : dob,
status : private.status
};
}
};
})("jerry", "mateparae", new Date("1954-11-14"));
And some calling code:
console.log(ns.person.getFullName());
console.dir(ns.person);
console.log(ns.person.capitaliseName("temuera morrison"));
ns.person.setStatus("governor general");
console.log(ns.person.getStatus());
console.log(JSON.stringify(ns.person));
And output:
Jerry Mateparae
{ getFullName: [Function],
capitaliseName: [Function],
getAgeInYears: [Function],
setStatus: [Function],
getStatus: [Function],
toJSON: [Function] }
Temuera Morrison
governor general
{"firstName":"jerry","lastName":"mateparae","fullName":"jerry mateparae","dob":"1954-11-14T00:00:00.000Z","status":"governor general"}
C:\src>
{ getFullName: [Function],
capitaliseName: [Function],
getAgeInYears: [Function],
setStatus: [Function],
getStatus: [Function],
toJSON: [Function] }
Temuera Morrison
governor general
{"firstName":"jerry","lastName":"mateparae","fullName":"jerry mateparae","dob":"1954-11-14T00:00:00.000Z","status":"governor general"}
C:\src>
OK, so this works, in that it hides the properties from the calling code. But - thanks to the IIFE approach - it just creates a one-off object. That's a pretty shit pattern, IMO: the "class" code is not at all reusable... if one wants a second person, one needs to repeat the code. It's also a wee bit shit cos if we want to change the state of any of the private values we need to call them something different from any argument name that might be used to pass-in the values. EG: one cannot do this:
setStatus : function (status) {
status = status;
},
One needs to do this (or some variation on this, eg have an
_status internal variable or something:ns.Person = function(firstName, lastName, dob, status){
var private = {
// ...
status : status
};
// ...
return {
// ...
setStatus : function (status) {
private.status = status;
},
// ...
};
};
It's not the end of the world, but it's all getting a bit "let's pretend we can do some stuff that we actually can't".
I don't quite know what the thinking was here. We're not gonna be using this approach.
The tight-coupling / non-reusable issue here is easily dealt with with some slight refactoring though: just get rid of the IIFE and give it a class-ish name, not an object-ish one (in this case: ns.Person instead of ns.person):
ns.Person = function(firstName, lastName, dob, status){
// ...
};
(I've elided everything that's not the same: so this is indeed not much of a problem to refactor).
But there's an architectural problem with all this.
Compare these two approaches:
Using JavaScript's traditional prototype approach:
ns.Person = function(firstName, lastName, dob, status){
// ...
};
ns.Person.capitaliseName = function(name){
return name.replace(/(^|\b)([a-z])/g, function(match){return match.toUpperCase();});
}
ns.Person.prototype.getFullName = function(){
return ns.Person.capitaliseName(this.firstName)
+ " "
+ ns.Person.capitaliseName(this.lastName);
};
Using the (refined) Module Pattern:
ns.Person = function(firstName, lastName, dob, status){
// ...
var capitalise = function(name){
return name.replace(/(^|\b)([a-z])/g, function(match){return match.toUpperCase();});
};
return {
getFullName : function(){
return capitalise(private.fullName);
},
// ...
};
};
Two things:
- this all still only creates an object; there's no concept of a class here, so one cannot have class methods, not object methods.
- more significantly, because of this, every time one creates a new instance of this, one recreates all the methods too! When using leveraging prototyping, the methods are just created once, an when creating a new object, the object simply uses the prototype.
Here's where I started demonstrating my lack of abilities in JavaScript though. I theorised I could hybridise their two approaches as still use both the private variables via closure, and apply the methods to the prototype at the same time. Way clever. Except it's not... let's have a look.
ns.Person = function(firstName, lastName){
var capitalise = function(name){
return name.replace(/(^|\b)([a-z])/g, function(match){return match.toUpperCase();});
};
var fullName = capitalise(firstName + " " + lastName);
ns.Person.prototype.getFullName = function(){
return fullName;
};
ns.Person.prototype.toJSON = function(){
return {firstName:firstName, lastName:lastName, fullName:fullName};
};
ns.Person.capitaliseName = capitalise;
};
I've simplified the requirement here as this is enough to demonstrate the functionality and the problem. The conceit here is that instead of returning just a plain object populated with inline functions, I still put the functions into the prototype. And as the prototyped functions are within the context of the constructor now, they also use closure to close over the local variables without them being exposed as public.
And with a very superficial test:
var person = new ns.Person("helen", "clark");
console.dir(person);
console.log(person.firstName);
console.log(person.lastName);
console.log(person.getFullName());
console.log(JSON.stringify(person));
console.log(ns.Person.capitaliseName("sian elias"));
Everything seems to work:
{}
undefined
undefined
Helen Clark
{"firstName":"helen","lastName":"clark","fullName":"Helen Clark"}
Sian Elias
C:\src>
undefined
undefined
Helen Clark
{"firstName":"helen","lastName":"clark","fullName":"Helen Clark"}
Sian Elias
C:\src>
The private properties are not exposed to the calling code, but they are accessible via the methods. And one can still have class methods too. Win.
Except it doesn't work. And it's obvious it doesn't work. I let myself down here as I've fallen into the "confirmation bias" trap, which is something I'm aware of, and usually know to not fall foul of, but I forgot about it this time. I looked for ways to demonstrate I was right, rather than look for ways to demonstrate I was wrong. When testing a new hypothesis, one needs to do both of these.
After getting the code ready for this article (well: a more triumphant version of this article, anyhow), the "confirmation bias" thing occurred to me, plus I remembered I'm rubbish at JavaScript and if this was an approach that worked, I'd not be the first to come up with it. So I googled and I stack-overflowed.
Yeah, people had tried this before, and had posted the relevant questions on SO. And the errors in their ways had been pointed out.
Initially only one problem came to light: I might be using the prototype here, but I'm re-setting the prototype method every time I create an object, given I'm defining them in the constructor. Duh. That was really stupid of me.
I still thought this was easy to overcome like this:
ns.Person = function(firstName, lastName, dob, status) {
this.dob = dob;
var private = {
firstName : firstName,
lastName : lastName,
status : status
};
// ...
if (!ns.Person.prototype.prototyped) {
// [prototyped methods here]
ns.Person.prototype.prototyped = true;
}
// ...
};
So only on the first definition of the constructor would the prototyped methods be defined. All good. Except... something was niggling at me. One of the Stack Overflow posts I read mentioned "you could end up referring to the wrong object instance if you take this approach" (this was actually the unrevised approach, without the check for prototyped being set). I didn't pay much attention to this initially, but then PHPStorm was also telling me "possible misusage of this". I decided to look closer. And pay attention to what I was doing.
First a more thorough test to see what I was doing:
console.log("The person is Jane Campion (b. 1954)");
var person = new ns.Person("jane", "campion", new Date("1954-04-30"), "not set");
console.dir(person);
console.log(person.getFullName());
console.log(person.getAgeInYears());
console.log("======================");
console.log("The person is Kerry Fox (b. 1966)");
var secondPerson = new ns.Person("kerry", "fox", new Date("1966-07-30"));
console.dir(secondPerson);
console.log(secondPerson.getFullName());
console.log(secondPerson.getAgeInYears());
console.log("Set Fox to active and verify:");
secondPerson.setStatus("Active");
console.log(secondPerson.getStatus());
console.log("======================");
console.log("Jane Campion serialised:")
console.log(JSON.stringify(secondPerson));
console.log("======================");
console.log("Kerry Fox serialised:")
console.log(JSON.stringify(person));
And the output (pay attention now):
The person is Jane Campion (b. 1954)
{ dob: Fri Apr 30 1954 01:00:00 GMT+0100 (GMT Summer Time) }
Jane Campion
62
======================
The person is Kerry Fox (b. 1966)
{ dob: Sat Jul 30 1966 01:00:00 GMT+0100 (GMT Summer Time) }
Jane Campion
50
Set Fox to active and verify:
Active
======================
Jane Campion serialised:
{"firstName":"jane","lastName":"campion","fullName":"Jane Campion","dob":"1966-07-30T00:00:00.000Z","status":"Active"}
======================
Kerry Fox serialised:
{"firstName":"jane","lastName":"campion","fullName":"Jane Campion","dob":"1954-04-30T00:00:00.000Z","status":"Active"}
C:\src>
{ dob: Fri Apr 30 1954 01:00:00 GMT+0100 (GMT Summer Time) }
Jane Campion
62
======================
The person is Kerry Fox (b. 1966)
{ dob: Sat Jul 30 1966 01:00:00 GMT+0100 (GMT Summer Time) }
Jane Campion
50
Set Fox to active and verify:
Active
======================
Jane Campion serialised:
{"firstName":"jane","lastName":"campion","fullName":"Jane Campion","dob":"1966-07-30T00:00:00.000Z","status":"Active"}
======================
Kerry Fox serialised:
{"firstName":"jane","lastName":"campion","fullName":"Jane Campion","dob":"1954-04-30T00:00:00.000Z","status":"Active"}
C:\src>
What's going on here then?
- both objects have the same name values
- but the correct DOB values
- and the active flag has been applied to both of them
It's so simply once I looked at it.
I was leveraging closure to get these "private" variables right? But the closure actually only takes place when the prototyped methods are defined, which is only when the first object is created. So all the methods point to Jane Campion's object (including the active value). The reason why the DOB still works is that I'm accessing this directly from the object, not the incorrectly-closured methods. So Kerry Fox's object still has all the right values internally, but the public methods all internally point to Campion's values, so Fox's values are all very private indeed: there's no way to expose them!
Note that the version of this code without the attempt to not redefine the prototype methods for each object will actually have the same problem in reverse: every new object the methods would be recreated to point to that object's properties. Meaning all earlier objects will also use those methods pointing to the latest object.
Sigh.
I gave up.
All this exercise was trying to achieve was to give us private properties and methods. But the advice from our coding standard was sound:
- acknowledge that JavaScript is OO but not in the sense we were used to (coming from CFML), and don't bother trying to pretend it's traditional OO by trying to do inheritance or non-supported stuff like that.
So my advice back to the team is that we should continue writing JavaScript they way it's prescribed to be written: using prototyping and accepting there's no such thing as a private property in JavaScript. And try to avoid me getting into situations where I try to be clever. I'm no good at it.
Not exactly the article I intended to write today.
Righto.
--
Adam
