I was recently reading a pretty well written article about Angular vs Backbone, and I noticed this little snippet of code:

Router = Backbone.Router.extend({
    routes: {
        '': 'phonesIndex',
    },
 
    phonesIndex: function () {
        new PhonesIndexView({ el: 'section#main' });
    }
});

Naturally the lack of a var keyword is generally a really bad thing (particularly in a tutorial), but whatever. Maybe he's one of those weird, archaic people like Crockford who still insist on putting all of their var declarations at the top of a function. Like a caveman.

But I digress.

The real annoying thing about that code snippet is the constructor that does nothing. Or rather, the constructor that does too much. That kind of code really only seems to crop up in JavaScript and PHP (from what I've seen), and I'm not completely sure if that correlation means something. I've also occasionally seen property accessors in C♯ that have side effects, which is not quite as terrible but still really stupid.

Constructors are eponymous in nature, meaning that their only job is to construct things. I mean, it's not even confusing, so it's pretty weird when people get it wrong.

If you read the next paragraph of the linked article, you'll see the following snippet, which explains the reason why nothing is done with the constructed object.

PhonesIndexView = Backbone.View.extend({
    initialize: function () {
        this.render();
    },
 
    render: function () {
        this.$el.html(JST['phones/index']());
    }
});

In Backbone, the initialize property doubles as a constructor. It's kind of weird, but it's how a lot of libraries handle classical inheritance. Basically, when new PhonesIndexView() is executed it internally calls PhonesIndexView.initialize(). So effectively initialize is a constructor.

The astute reader will also notice the this.render() call inside initialize, which, not surprisingly, renders the view onto the page. Hence the reason for not doing anything after constructing the object: the constructor does all the relevant work.

There are a few objective reasons why this is bad:

1. It violates the purity of a constructor.
2. It makes the code harder to read and understand.

Purity

A pure function or method is one that doesn't have side effects. So if your function does modify state (e.g. add or delete something), then your function is no longer pure. Note that it's not code smell if a function is not pure; in fact, it's kind of impossible (or at least extremely annoying) to write completely pure code.

Constructors, however, should always be pure. They shouldn't be establishing network connections (e.g. connecting to a database), or modifying state (e.g. rendering a view); they should just declare some local variables and exit. Methods and functions on the constructed object should be the ones doing things, not the constructor.

Take this example: which code do you like better?

Pure constructor

function Database(credentials) {
  this.username = credentials.username;
  this.password = credentials.password;
  this.conn = null;
}

Database.prototype.connect = function() {
  if (!this.conn) {
    this.conn = someDatabaseLibrary.connect(this.username, this.password);
  }
};

var db = new Database({
  username: 'tmont',
  password: 'heartbleed'
});

try {
  db.connect();
} catch (e) {
  console.error('failed to connect');
}

Constructor with side effects

function Database(credentials) {
  this.username = credentials.username;
  this.password = credentials.password;
  this.conn = someDatabaseLibrary.connect(this.username, this.password);
}

try {
  var db = new Database({
    username: 'user',
    password: 'heartbleed'
  });
} catch (e) {
  console.error('failed to connect');
}

The difference between the two is that the connection in the second example occurs implicitly. The constructor has side effects: it's attempting to connect to the database server. This makes it harder to pinpoint errors (say the db server is down) because too many things occur by magic. If you explicitly call connect(), and an exception is thrown, it's not too hard to deduce there is a problem with trying to connect. You wouldn't expect merely invoking a constructor to give a connection error.

And if one side effect occurs in the constructor, why not more? May as well do some logging while we're in there (hopefully the log file is writable and the disk isn't full, or there will be more exceptions), or send an email notification (hopefully the email server is up!).

And suddenly you've got code that looks like this:

function Database(credentials) {
  this.username = credentials.username;
  this.password = credentials.password;
  this.logger = Logger.get('myApp');

  this.logger.debug('attempting to connect to db with username ' + this.username);
  this.conn = someDatabaseLibrary.connect(this.username, this.password);
  this.logger.debug('successfully connected to db');

  var connections = parseInt(this.conn.get('dbstats:connect')) + 1;
  this.conn.set('dbstats:connect', connections);

  if (connections % 100 === 0) {
    var emailer = new Emailer('emailserver.com:25');
    emailer.send(
      'foo@example.com', 
      'Database connection update', 
      'Connected to db server ' + connections + ' times'
    );
  }
}

There are so many areas of possible failure that it's impossible to keep it straight in your head. Particularly if you have a codebase full of code like this.

The Single Responsibility Principle is usually in reference to classes: each class should have a single responsibility. But functions and methods should also follow that rule. If you have a method called connect() it shouldn't do anything but connect.

JavaScript is kind of a weird entity, since it's not classical, but is often written classically for larger applications. For this reason, large JavaScript code bases can get out of hand very quickly and turn into giant piles of spaghetti if you're not careful. Following best practices and design/architecture patterns can alleviate a lot of mess with very little effort.