3
\$\begingroup\$

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

\$\endgroup\$
  • \$\begingroup\$ Can you give an example of how you'd use this? I can't quite see a situation where it'd be super helpful, to be honest. \$\endgroup\$ – Flambino Oct 26 '14 at 21:27
  • \$\begingroup\$ Sure. I've got a menu that will trigger content being loaded in a page div. Sometimes that content is a Wizard, which uses the session as a conversation scope. Clicking on a separate menu option that only uses .empty() may not perform cleanup, such as cleaning up the session, custom events, etc. \$\endgroup\$ – Jason Oct 26 '14 at 22:21
5
\$\begingroup\$

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();
    });
}
\$\endgroup\$
  • \$\begingroup\$ I was thinking the same, but the function actually invokes the callback before it empties the element \$\endgroup\$ – Flambino Oct 26 '14 at 22:31
  • \$\begingroup\$ ^ Ah, good spot. See my updated answer. :-) \$\endgroup\$ – Ben Oct 26 '14 at 22:37
  • \$\begingroup\$ Thank you. My concern was that the callback would contain asynchronous code that needed to complete before the empty operation. My main concern was that I was doing the deferred incorrectly. \$\endgroup\$ – Jason Oct 26 '14 at 23:51
4
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.