Refactoring asynchronous JS pre-rendering code

Question

A few months ago I wrote this module but, coming back to it, I find it a bit hard to read and reason about. I want to ask community's opinion on whether this needs to be refactored, and how I could approach this.

(function (q) {

  'use strict';

  var DELAY_BEFORE_RENDERING_NEXT_PAGE_VIEW = 3 * 1000;

  window.st.viewer.Helper.PageViewFactory = (function () {
    function promiseRenderView(page, token) {
      return page.fetchUnlessReady()
        .then(token.throwIfCanceled)
        .then(function () {
          var view = new window.st.viewer.View.PageView({
            model: page
          });

          view.render();
          return view;
        });
    }

    var scheduledPage,
        scheduledPromise;

    function scheduleRenderNextView(page, token) {

      // After some time passes, prefetch and prerender next page view
      // so if it is requested (very likely), we can re-use an existing promise.

      scheduledPage = null;
      scheduledPromise = null;

      q.delay(DELAY_BEFORE_RENDERING_NEXT_PAGE_VIEW)
        .then(token.throwIfCanceled)
        .then(function () {
          return page.promiseNext();
        })
        .then(token.throwIfCanceled)
        .then(function (nextPage) {
          if (nextPage) {
            scheduledPage = nextPage;
            scheduledPromise = promiseRenderView(nextPage, window.st.Helper.CancellationToken.getNone());
          } else {
            scheduledPage = null;
            scheduledPromise = null;
          }
        })
        .catch(token.catchItself)
        .done();
    }

    function scheduleRenderView(page, token) {
      var promise = (page !== scheduledPage) ?
        promiseRenderView(page, token) :
        scheduledPromise;

      scheduleRenderNextView(page, token);

      return promise;
    }

    return {
      scheduleRenderView: scheduleRenderView
    };
  })();
})(Q);

This module exports a single function called scheduleRenderView. Its purpose is, given a page model and a cancellation token, do the following steps asynchronously:

1. Fully fetch page model (it may be loaded partially);
2. Create and render (in memory) a PageView for it;
3. Return this view to the caller;

4. Additionally, after a 3 second delay:

   • Request next page by calling promiseNext;
   • Pre-render next page by repeating steps 1-3 for it and store the pre-rendered view in case it gets requested the next time scheduleRenderView is called (very likely)

The user views pages one by one, and I wanted to anticipate the most likely scenario where she will request the next page.

I also wanted to make this optimization invisible to the calling code.

Is this code hard to comprehend? Are the simpler ways to achieve the same?

Answer 1

Is this code hard to comprehend? Yes.

That is because there are a lot of unknown functions: view.render, page.fetchUnlessReady, token.catchItself ,token.throwIfCanceled or window.st.Helper.CancellationToken.getNone() etc. etc.

In my mind, I would approach this more like :

function scheduleRenderView( page )
{
  //Load and display the page
  var promise = model.loadPage( page ).then( function(){
    view.renderPage( model.getPage( page ) );
  });
  //Load the next page
  model.loadPage( page.next() );
  return promise;
}

loadPage would either return immediately if we retrieved already the data once ( from cache ) or download the page model.

getPage gets a page model from cache

renderPage would do the obvious

This means I would forego the pre-rendering which really sounds like pre-mature optimization, how long could that take?

2 other minor comments:

• Why access some variables through window, that does not seem to make sense
• In my mind, adding functions to the window.st.viewer.Helper is terrible global variable abuse, not to mention that you probably should add it to Helper.prototype.

Answer 2

Two more points to complement konijn's answer to my question:

• I'm moving from window.st.* madness to using require.js with Almond (I wish I did this before);
• Instead of hand-rolled cancellation mechanism, I'm moving from Q to bluebird which already supports opt-in cancellation.