<glacius:metadata>

    <title>Constructors should not have side effects</title>

    <description>Blog post about concerning constructing objects without side effects</description>

    <category>Legacy blog posts</category>

    <category>Programming</category>

    <category>JavaScript</category>

</glacius:metadata>

<glacius:macro name="legacy blargh banner">

    <properties>

        <originalUrl>https://tmont.com/blargh/2014/4/constructors-should-not-have-side-effects</originalUrl>

        <originalDate>2014-04-17T05:54:22.000Z</originalDate>

    </properties>

</glacius:macro>

<p>

  I was recently reading a pretty well written article about 

  <a href="http://blog.42floors.com/coded-angular-tutorial-app-backbone-took-260-code/">Angular vs Backbone</a>,

  and I noticed this little snippet of code:

</p>

<glacius:code lang="javascript"><![CDATA[

Router = Backbone.Router.extend({

    routes: {

        '': 'phonesIndex',

    },

    phonesIndex: function () {

        new PhonesIndexView({ el: 'section#main' });

    }

});

]]></glacius:code>

<p>

  Naturally the lack of a <code>var</code> 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 <code>var</code>

  declarations at the top of a function. Like a caveman.

</p>

<p>

  But I digress.

</p>

<p>

  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.

</p>

<p>

  Constructors are eponymous in nature, meaning that their only job is to

  <em>construct things</em>. I mean, it's not even confusing, so it's pretty weird

  when people get it wrong.

</p>

<p>

  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.

</p>

<glacius:code lang="javascript"><![CDATA[

PhonesIndexView = Backbone.View.extend({

    initialize: function () {

        this.render();

    },

    render: function () {

        this.$el.html(JST['phones/index']());

    }

});

]]></glacius:code>

<p>

  In Backbone, the <code>initialize</code> property doubles as a constructor. It's

  kind of weird, but it's how a lot of libraries handle classical inheritance.

  Basically, when <code>new PhonesIndexView()</code> is executed it internally calls

  <code>PhonesIndexView.initialize()</code>. So effectively <code>initialize</code>

  is a constructor.

</p>

<p>

  The astute reader will also notice the <code>this.render()</code> call inside

  <code>initialize</code>, which, not surprisingly, renders the view onto the page.

  Hence the reason for not doing anything after constructing the object: <strong>the

  constructor does all the relevant work</strong>.

</p>

<p>

  There are a few objective reasons why this is bad:

</p>

<ol>

  <li>

    It violates the 

    <a href="https://www.artima.com/articles/never-call-virtual-functions-during-construction-or-destruction">purity</a>

    of a constructor.

  </li>

  <li>It makes the code harder to read and understand.</li>

</ol>

<h3>Purity</h3>

<p>

  A <em>pure</em> 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.

</p>

<p>

  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.

</p>

<p>

  Take this example: which code do you like better?

</p>

<h4>Pure constructor</h4>

<glacius:code lang="javascript"><![CDATA[

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');

}

]]></glacius:code>

<h4>Constructor with side effects</h4>

<glacius:code lang="javascript"><![CDATA[

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');

}

]]></glacius:code>

<p>

  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 <code>connect()</code>,

  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.

</p>

<p>

  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!).

</p>

<p>

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

</p>

<glacius:code lang="javascript"><![CDATA[

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'

    );

  }

}

]]></glacius:code>

<p>

  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.

</p>

<p>

  The <a href="https://en.wikipedia.org/wiki/Single-responsibility_principle">Single Responsibility

  Principle</a> 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 <code>connect()</code>

  it shouldn't do anything but connect.

</p>

<p>

  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.

</p>