Refactoring Under Test with Fab.js

‹prev | My Chain | next›

I refactored and DRYed up the fab.js code the in the backend of my (fab) game last night. I ended up with a robust upstream app, named unary_try, that invoked a callback to broadcast state changes to all players in the game then closed the downstream connection.

It seems to work well enough, but I have no tests—something that I ought to have given that I have 3 resources using it. It could also stand a little clean-up—something that tests would help as well.

The first test is easy enough—if the request contains only headers, then I should ignore it:

    function
    terminatesIfHead() {
      var ret;
      function downstream (obj) { ret = obj; }

      var upstream_listener = unary_try.call(downstream);
      upstream_listener({head: "foo"});

      this(typeof(ret) == "undefined");
    }

Here, I am simply hoping that the downstream (close to the browser) app is called with an empty argument. That test passes straight out. For something a bit more complex, I try to verify that the callback is called with the body POSTed downstream:

  var request = { body: "foo-bar" },
      body = null,
      callback = function() {body = arguments[0];};

    function
    invokesCallbackIfBody() {
      function downstream () { }

      var upstream_listener = unary_try.call(downstream);
      upstream_listener(request);

      this(body == "foo-bar");
    }

That test fails however. It takes me a sad amount of time and way too many puts statements to realize that I am not supplying the callback to the unary_try, but I ultimately get the test passing with:

    function
    invokesCallbackIfBody() {
      function downstream () { }

      var upstream_listener = unary_try(callback).call(downstream);
      upstream_listener(request);

      this(body == "foo-bar");
    },

In addition to these two tests, I add three others verifying that:

invokesCallbackIfBody: true
terminatesEvenIfBody: true
terminatesCleanlyOnException: true
terminatesIfHead: true
terminatesIfEmptyRequest: true
Done. 5 passed, 0 failed.

With that, I believe that I have fully covered the features of unary_try. Now I am ready to refactor it:

function unary_try(fn) {
  return function() {
    var out = this;
    return function listener( obj ) {
     if ( !obj ) out();
     else if ( obj.body ) {
       try {
         if ( obj && obj.body ) {
           fn.call(obj.body, obj.body);
         }
       } catch (e) {
         puts("[unary_try] error type: " + e.type +
              ", message: " + e.message);
       }
       finally {
         out();
        }
      }
      return listener;
    };
  };
}

I do not believe that I need the initial conditional checking for the POST value (covered by the terminatesIfEmptyRequest test). The finally block ought to cover it. So I refactor to:

function unary_try(fn) {
  return function() {
    var out = this;
    return function listener( obj ) {
      try {
        if ( obj && obj.body ) {
          fn.call(obj.body, obj.body);
        }
      }
      catch (e) {
        puts("[unary_try] error: " + e.message +
                              ' (' + e.type + ')');
      }
      finally {
        out();
      }
      return listener;
    };
  };
}

My tests still pass I have eliminated all but one conditional (and that one is mostly a convenience):

Running 5 tests...
invokesCallbackIfBody: true
terminatesEvenIfBody: true
terminatesCleanlyOnException: true
terminatesIfHead: true
terminatesIfEmptyRequest: true
Done. 5 passed, 0 failed.

That's a good stopping point for tonight. Up tomorrow: investigating a bug in collision detection when chatting.

Day #140