Unit testing has made me a better programmer

unit-testing.md

This post is blatant rip-off of the post Backbone has made me a better programmer by Andy Appleton. His message is excellent and I encourage you to read it, but I wanted to show that writing well structured and decoupled code is something that can be learned from unit testing as well as Backbone. Unit testing will force you to write good code and this is a very strong reason why you should be doing it!

I started unit testing about a year ago and have since used it on large and small projects at work and for fun.

These last two months I have been refactoring some JavaScript on ShareLaTeX and I was really surprised at the state of the code I had written not all that long ago. I have been rewriting it to use a number of design patterns that I have necessarily picked up from unit testing.

One object one responsibility

We all write clean encapsulated object oriented JavaScript right? Guys? No me neither – it’s so easy to just fire off an event, nest a few levels of callbacks and be done with it.

Here’s an example. We get some data via an xhr request, append it with a fade effect and then remove some other element.

$.ajax({
  url: '/wherever',
  success: function(data, status, xhr) {
    $('selector').append($(data).hide().fadeIn(function(){
      $('other-selector').remove();
    }));
  }
);

This is simple to write initially, but it becomes very tricky to unit test. Say we want to check that this code does what it should. Our test would need to look something like (in Coffeescript):

describe "Ajax call", ->
    beforeEach ->
        # This is a bit of an ugly mock don't you think?
        $.ajax = (options) ->
            options.success(@data = "Test data", 200, {})

    it "should insert the response data", ->
        $('selector').text().should.equal @data

    it "should remove other-selector", ->
        $('other-selector').length.should.equal 0

Not too bad yet, but say we also want to update another part of the document with a notice to say that the request was successful:

$.ajax({
  url: '/wherever',
  success: function(data, status, xhr) {
    $('selector').append($(data).hide().fadeIn(function(){
      $('other-selector').remove();
    }));
    $('yet-another-selector').text('Request succesful');
  }
});

That success callback is gradually getting to be responsible for a lot of different things and is starting to get pretty tricky to read. This only gets worse as other developers come to the code and just need to add a small bit of extra functionality. I find myself cursing them later for the messy code when actually it’s my fault for writing it this way in the first place. Our unit tests will also grow in a similarly messy way with one test having to check a lot of things:

describe "Ajax call", ->
    beforeEach ->
        $.ajax = (options) ->
            options.success(@data = "Test data", 200, {})

    it "should insert the response data", ->
        $('selector').text().should.equal @data

    it "should remove other-selector", ->
        $('other-selector').length.should.equal 0

    it "should update yet-another-selector", ->
        $('yet-another-selector').text().should.equal "Request successful"

Here’s a much nicer way to deal with this:

function DataModel() {
  this.url = '/wherever';
}

DataModel.prototype.getData = function() {
  $.ajax({
    url: this.url,
    context: this,
    success: this.onSuccess
  });
};

DataModel.prototype.onSuccess = function(data) {
  $(window).trigger('DataModel:success', data);
};

var dataModel = new DataModel();

Now we have a dataModel object which is concerned only with making the request. It triggers an event when it is done which we can listen for in other objects.

function ListView(el) {
  this.$el = $(el);
  this.bindEvents();
}

ListView.prototype.bindEvents = function() {
};

ListView.prototype.addData = function(data) {
  this.$el.append($(data).hide().fadeIn(this.removeOtherThing));
};

ListView.prototype.removeOtherThing = function(data) {
  this.$el.find('other-selector').remove();
};

var listView = new ListView('selector');

Right now we have more lines of code, but what we also have is two separate objects each performing a discrete function. Each object has a number of methods and the methods each do one thing. It's much easier to write unit tests for this code and each unit test will be smaller and with a clear point:

describe "ListView", ->
    beforeEach ->
        $(window).trigger('DataModel:success', @data = "Test data");

    it "should insert the response data", ->
        $('selector').text().should.equal @data

    it "should remove other-selector", ->
        $('other-selector').length.should.equal 0

describe "DataModel", ->
    # Well separated tests...

So now when we needed to add another callback to the AJAX completion all we need to do is create a new object type which will listen for the same DataMode:success event and do its thing independently of our listView. That’s pretty awesome and it means that our original code can stay responsible for the one task it was designed.

Thanks unit testing!

This is now well structured code but it’s often easy to take the (seemingly) quicker option. Unit testing makes you do right first time.

I’m spending more time up front thinking about the structure and maintainability of my code in the expectation that it will prevent it from getting to that tangled spaghetti-mess that we all hate coming back to and that is definitely a good thing.