Why these jQuery worst practices aren’t.

jQuery … Worst Practices

In this post, Steve Wellens tries to make the case for two common patterns your run across when using jQuery as “worst practices”. Practices that are either superfluous or harmful to your code.

1) Wiring up events using unobtrusive JavaScript.

Instead of wiring up your elements events using jQuery, instead you should set the OnClientClick property of the ASP.NET Web Control in question and call whatever JavaScript you want to handle the event.

Instead of this:

    <script type="text/javascript">

        $(document).ready(function()
        {
            $("#Button1").click(function(event)
            {
                alert("You Clicked Me!");
            });
        });

    </script>

Do This:

    <asp:Button ID="Button1" 
                runat="server" 
                Text="MyButton"                
                OnClientClick="alert('You Clicked Me!');" /> 

His reasoning is as follows:

You might want to change event handlers dynamically depending on some condition in the page. If you need to assign a common event handler to several objects on a page, it makes sense to do it in one place. As a matter of fact, that’s the beauty of jQuery. But for a single event on a single control….NO.

On can debate whether or not a worst practice can really be applied to a single, specific , one-time event or not. But there are other reasons why you might want to hook up events to a single element using jQuery. one being that you can’t hook up multiple event handlers using OnClientClick or HTML’s OnClick event, only a single event handler can be assigned. If you are using the observer pattern in your JavaScript, you may have multiple observers that want to subscribe to your buttons click event. But a really strong argument could be made for using the OnClientClick property simply due to ASP.NET’s suck-ass way of handling client IDs.

2) Chaining can lead to unreadable code.

In a badly written example, he states that chaining calls to jQuery makes the code unreadable. I’d suggest re-formatting the code in this manner to make it legible and allow for a tighter minimization.

$("div.highlight")
  .css("color", "yellow")
  .css("background-color", "black")
  .css("width", "50px");

Of course, you wouldn’t write jQuery code like that and call the css method multiple times. Ideally, you would just set a class or at the very least pass in a set of properties in an object.

$("div.highlight").css({
  color:"yellow",
  background-color:"black",
  width:"50px"
});

3) Comment your code.

// get all the Divs with the highlight class and
// format them and set their width

var Divs = $("div.highlight");
Divs.css("color", "yellow");
Divs.css("background-color", "black");
Divs.css("width", "30px");   

I like this explanation by Jeff Atwood best. If you need comments to explain WHAT your code is doing, you need to use better names and/or write better code. Comments are best when used sparingly and explain the WHY of the code. Why did you choose to use a particular algorithm

  • I fully agree. I didn’t quite get why he called those bad practices. AFAIK, the things he mentions as bad practices are actually the way jQuery is designed/what jQuery is designed for.

    Especially when you have one guy doing the markup and another doing the scripting, rule #1 is really important. You can let someone simply change the view, without breaking the scripting (as long as you keep the same names/IDs of course).

  • Nitpicking:

    $(“div.highlight”).css({
    color:yellow,
    background-color:black,
    width:50px
    });

    should be:

    $(“div.highlight”).css({
    color:”yellow”,
    background-color:”black”,
    width:”50px”
    });

  • Ryan Roberts

    His argument against method chaining was particularly gruesome. That might be something to do with his confessed inexperience with javascript. Otherwise he would likely have a deeply ingrained hatred of imperative nodeset twiddling.

    I asked him whether he would assign to intermediate results to make his SQL ‘clearer’, or whether he would use the language to declaratively manipulate sets as intended.

    And he blocks comments, most unsporting.

  • HA, good catch Duncan. I’ll update the post, otherwise that’ll bug me too.

  • ken

    I’m only going to address #2.

    First, I think his chosen example was just that, an example. Take some “real-world” examples, where one might use .css(), .click(), .show(), .hover(), etc. all on one chain.

    That being said, I think I agree with the original author though, and not you. I would bet money that his method is less error-prone when maintained — that is, unless you all are elite coders whose code never needs to be maintained, (even after a change in requirements, refactoring, etc)?

    So what you’re saying is, even if you have 50 calls, they should all be on the same “chain”? Have you ever written a chain longer than 3-4 calls? It gets even more fun when you try to inline anonymous functions and such.

    Breaking up a chain lets you reorganize the chain easier. For example, say you have a long chain, and at several points you filter your collection by using .find(), do some operations, and then you return to your original collection by calling .end(). This is a maintainablity (and readability) nightmare! Sure, it may be easy for you, but reading code is like reading handwriting: its always so much easier when its your own. The poor sap who comes along after you isn’t going to have a very fun time.

    Breaking chains is also easier to debug. How are you going to console.log that intermediate value? How are you going to add a debugger statement right before the ajax call?

    Lastly, breaking up chains would also be wise in the name of safety. Whitespace can cause all sorts of very hard to diagnose issues, including the dubious semicolon insertion problems and minifier/packer compatibility issues.

    The fluent interface design pattern if immensely powerful, however, it can also let you shoot yourself in the foot very fast.

  • “So what you’re saying is, even if you have 50 calls, they should all be on the same “chain”?”

    What I said was that there was a way to make the chaining readable. I’d consider a jQuery call that contained 50 chained call to be a code smell. But if you are binding several events to the same set of elements defined by a single selector, chaining the calls is a valid choice.

    IMO, once you’re inside of jQuery any kind of debugging increases in degree of difficulty.

  • Andrea

    Please use a spell checker before posting articles, Scott.

  • LMAO@Andrea

    Hah too funny Andrea, being critical of spelling on a blog domain entitled the “lazycoder”. Wow uptight much? 🙂

  • Steve Wellens

    Straw man arguments are tedious to refute.

    For the record, my only problem with certain practises is that they imply, “Do it this way all the time.” There are pros and cons when choosing a particular solution to a particular problem. Slavishly following a paradigm does not always result in an optimum solution.