Wrapping jQuery empty() To Include A Callback

Question

I needed to write a method to extend the jQuery empty() function to include a callback. Since I didn't want to mess with namespaces, closures, etc, I instead wrote a regular function to wrap empty().

var emptyWithCallback = function(node, callback) {
    console.log('Entered emptyWithCallback()');
    if (typeof callback != 'function') {
        return false;
    }
    var doTheCallback = callbackWrapper();
    var doTheEmpty = myEmpty();

    function callbackWrapper() {
        var deferredCallback = $.Deferred();
        callback();
        deferredCallback.resolve();

        return deferredCallback.promise();
    }

    function myEmpty() {
        var deferredEmpty = $.Deferred();
        $(node).empty();
        deferredEmpty.resolve();

        return deferredEmpty.promise();
    }

    doTheCallback.then(doTheEmpty);
}

I was attempting to use the $.Deferred() in case the callback contained an ajax call, in order to not execute the .empty() until the callback concluded.

Does this code look like it will work? It seems to work on simple callbacks, such as a single alert() or console.log().

Answer 1

I can't really see how wrapping empty() with a $.Deferred object manages to achieve anything; after all empty() executes immediately and doesn't need the consumer to wait for a function callback.

Instead think of how you might be using empty() with an asynchronous context and then abstract that. For example if it is an AJAX request which you need to empty an element out, then fetch, then display the new data your implementation might look like this:

var clearAndThenFetch = function($element, ajaxOptions, callback) {
    var deferred = $.Deferred();

    $element.empty();

    $.ajax($.extend({
        success: function(response) {
            callback && callback(deferred, response) || deferred.resolve();
        },
        error: function(xhr) {
            deferred.reject(JSON.parse(xhr.responseText).errors);
        }
    }, ajaxOptions));

    return deferred.promise();
};

Otherwise I can't really see the need for abstraction here.

Edit: In regards to Flambino's comment, why do you need the empty() deferred at all? Remember that DOM operations are synchronous and so don't need to use promises. With this in mind we can rewrite your function like so:

var emptyWithCallback = function(node, callback) {
    console.log('Entered emptyWithCallback()');
    if (typeof callback != 'function') {
        return false;
    }

    var doTheCallback = function() {
        var deferredCallback = $.Deferred();
        callback();
        deferredCallback.resolve();

        return deferredCallback.promise();
    }

    doTheCallback.then(function() {
        $(node).empty();
    });
}

Answer 2

Took me a while to understand the order of things here. I expected the function to empty an element, and then invoke the callback. This would be the conventional approach: I kick off some process, and use a callback to let us know when it's done.

In the comments, I asked where this might come in handy, and while I got an example, I'm still not sure it's necessary to create this wrapper for it.

Calling empty is literally just .empty(). I would think that that can be done where ever you might need it. For instance, I don't see any huge difference between:

// do some things to clean up, and then...
someElement.empty();

and

emptyWithCallback(someElement, function () {
  // do some things to clean up
});

So really, I don't see any great benefits here.

However, there's still code to review:

• As @Ben points out, wrapping DOM operations in promises is rather pointless, since DOM operations are synchronous.

• Moreover, wrapping the callback in a promise is also pointless. Even if the callback is an asynchronous operation, you still call resolve() right away.

  If the point is to wait for the callback to finish, then you should do something else. Right now, if the callback starts something asynchronous, you'll just empty the element before the asynchronous operation has finished.

  So right now, your function can be written as

  var emptyWithCallback = function(node, callback) {
      if(typeof callback !== 'function') return false;
      callback();
      $(node).empty();
  }
  

  That's functionally identical to what you're doing now. 3 lines.

• Your check of callback is a little worrisome. You more or less fail silently if it isn't. Sure you return false, but your function doesn't return true in the opposite case; it returns undefined which is also false'y.

• Oh, and don't leave console.log lines in production code.

So again, I fail to see any real benefit to doing this.