4
\$\begingroup\$

What are your guys thoughts on the following jQuery plugin?

$.fn.with = function( fn ) {
    // Verify function
    if( jQuery.isFunction( fn ) ) {
        var temp = fn.call(this);

        // Check fn for valid response
        if( temp instanceof jQuery ) {
            return temp;
        }
    }
    return this;
};

I believe this method to very useful as it allows for better chaining, thus increasing flow. Before developing this method, I found myself creating a lot of temporary variables just to preform a conditional test.

Examples would be in cases where .hasClass and .is() are used:

// Example From: http://api.jquery.com/is/

$("ul").click( function( event ) {
    var $target = $(event.target);

    if ( $target.is("li") ) {
        $target.css("background-color", "red");
    }
});

However, using the proposed .with() method, the code can become cleaner:

$("ul").click( function( event ) {
    $(event.target).with(function() {
        if( this.is("li") ) {
            this.css("background-color", "red");
        }
    })
});

Note: If the function processed by .with() returns an instance of jQuery it, instead of the calling instance, will be returned into the chain.

\$\endgroup\$
  • 2
    \$\begingroup\$ The resulting code is an order of magnitude uglier, even taking an extra indentation level. \$\endgroup\$ – Esailija Jul 20 '13 at 16:19
  • \$\begingroup\$ I would suggest you submit that to jQuery. It seems like a viable method and might be added to the core. \$\endgroup\$ – Jonny Sooter Jul 30 '13 at 22:20
  • \$\begingroup\$ Do you know what the process is for that or a site for reference? I've not found anything at www.jquery.com. \$\endgroup\$ – roydukkey Aug 16 '13 at 15:43
2
\$\begingroup\$

The code

I'm not quite sure of the purpose of this part:

if( temp instanceof jQuery ) {
    return temp;
}

This seems to be related to the idea of chaining methods, but it prevents me from doing something like this:

var width = $('ul').show().with(function() {
    return this.width();
}

Also, when chaining, there is no benefit of only returning the result if it is an instance of jQuery, because if you try to chain methods and you return nothing from with with(), it'll result in "undefined is not a function".

You also check if the fn passed to with() actually is a function. I wouldn't do that, since this will silently ignore a wrong parameter to with(), which is very hard to debug. Instead you can just disable the check, or even better do something like this:

if(jQuery.isFunction(fn)) {
    [...]
} else {
    throw "Parameter passed to `with()` was not a function: " + fn;
}

The idea in general

I have to side with @Esailija here. While there won't be a noticeable performance difference in any modern browser (if you omit the instanceof check), I can't see the benefit of replacing a local variable with an extra level of unnecessary indentation. But that is obviously a purely personal opinion.

Aside from the extra level of indentation, I find the code using with() harder to read, since it removes context from the individual lines. I would prefer something like

$targetEl.css("background-color", "red");

to

this.css("background-color", "red");

any day, because in the former I don't have to scan the surrounding code to see what "this" currently means (which can be hard in JavaScript). Of course this is no concern in a snippet as short as your examples.

\$\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.