Steve Wellens

Programming in the .Net environment

Sponsors

Links

jQuery…Worst Practices

jQuery and Regular Expressions have a lot in common: They are both amazingly powerful and they are both as ugly as a knife fight between two obese street-whores in a garbage-strewn alley.

There isn't much hope for Regular Expressions. They are what they are. I use a freeware program called Expresso which can parse regular expressions and provide text descriptions for them.

But for jQuery, there is hope. You can use this powerful tool and still create code that is readable and maintainable. It takes a bit more work, and a bit more thought, but it is worth it.

Caveat: I am not a jQuery expert; I am not a JavaScript expert. Not even close. But I know the difference between good code and bad code: Good code is easy to understand.

 

Worst Practice #1:  Hooking up event handlers dynamically when there is no need.

In the document ready event, some developers search for a DOM object and connect an event handler to it as follows:

<script type="text/javascript">
    $(document).ready(function()
    {
        $("#Button1").click(function(event)
        {
            alert("You Clicked Me!");
        });
    });
</script>

Why add indirection? Why move the event hookup away from the object? It is much more straightforward and object oriented to do it the "old fashioned" standard way:

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

Of course, there are reasons for doing it with jQuery.  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.

Just because you can do something doesn't mean you should do it.

 

Worst Practice #2:  Chaining the results.

A touted feature of jQuery is the ability to chain results as in this example:

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

Let's make it more readable and more maintainable. It's easier to work with many short lines than a single monolithic monstrosity:

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

Isn't that better? In the real world, hopefully you would use a CSS class to format the divs.

By the way, putting everything on one line does not make it run faster.  This is so important I'm going to repeat it:

Putting everything on one line does not make it run fasterDon't believe me? Here's proof:

$(document).ready(function()
{
    for (i = 0; i < 1000; i++)
    {
        Test1();
        Test2();
    } 
});
void function Test1()
{
    $("div.highlight").css("color", "yellow").css("background-color", "black").css("width", "50px");
}
void function Test2()
{
    var Divs = $("div.highlight");
 
    Divs.css("color", "yellow");
    Divs.css("background-color", "black");
    Divs.css("width", "50px");
}

Here are the results from the IE 8 Profiler:  The one-line code took longer. 

If you run the test several times, the results vary slightly but the conclusion is that there is no performance gain chaining lines together...there is only a loss of maintainable code.

 

Worst Practice #3:  Lack of comments.

jQuery is cryptic. If any code ever screamed out for comments it is jQuery code. Let's raise the professionalism of the above 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");                        

Isn't that better than the original cryptic single-line? It doesn't take that much extra effort to produce good code.


If you work with technically proficient developers, you'd better not write sloppy code or you'll find yourself reviewing documentation in a tiny cubicle next to a loud-voiced sales person who uses their speaker phone for everything…including checking voice mail.

If you work for a large, non-technical company, chances are that no one gives a rat's ass what you do and you can get away with writing sloppy code.

If you work by yourself in a small company, no one is going to be evaluating your code and you can get away with writing sloppy code.

However, hopefully, a sense of self-pride will compel you to write good code….not just code that 'works'.

 

In summary, jQuery is great. It can save a lot of time in initial development. But if you don't take care, the productivity gains will be lost by having code that is difficult to understand and therefore difficult to maintain.

 

// send friendly message to all jQuery Developers 
$("jQuery.Developers").HappyProgramming();
 

Steve Wellens

Comments

Frederik Vig said:

Worst Practice #2

[quote]$("div.highlight").css("color", "yellow").css("background-color", "black")[/quote]

Can be written:

[code]

$("div.highlight")

   .css("color","yellow")

   .css("background-color", "black")

   .css("border", "1px solid black");

[/code]

Which imho is pretty readable.

Worst Practice #3

Imho that code explains itself pretty well, comments like in your example are then unnecessary. Though I agree that complex code should be commented, but again it might be a better idea to clean up the code a little if it is to complex to read and understand?

# May 9, 2009 7:41 AM

SGWellens said:

Yes, it's more readable but it's still not as maintainable as it could be.  If there is a an error, having separate statements is better because you can quickly isolate what line the error is on.

Even if the code is very readable, comments are important because they help developers more quickly find the code they are looking for.

# May 9, 2009 8:56 AM

SGWellens said:

I know the example is contrived, I pointed out that it would be better to use CSS to style an element.

It's OK to give an element a non-existant class so you can access it later with a selector.  I read this somewhere so I know it's true :)

# May 9, 2009 10:09 AM

Jeff said:

I disagree with your first and third points. For people who prefer unobtrusive Javascript, especially in shops where specific people do specific things, it's absolutely better to wire up events away from the DOM itself. The HTML jockey can edit in a low-risk manner.

As for your third example, fine, make comments, but you better have something in your build process that strips them out later. I don't want to pay for those bits on a busy site.

# May 9, 2009 10:17 AM

SGWellens said:

>> I disagree with your first and third points.

Nah, we do agree:

I pointed out there were many times when it IS appropriate to dynamically hookup up event handlers.  My point was there are times when it is NOT appropriate.

I agree with your second point.  If performance is critical, strip out stuff...including white space.

# May 9, 2009 10:43 AM

CurlyFro said:

i completely disagree with #1.  when building big websites, it's critical to isolate/separate code from UI.

# May 9, 2009 10:47 PM

SGWellens said:

No, you don't completely disagree with #1.  You qualified your statement to be for big websites only...and thats fine. 

But, other times, it's not critical and can add unnecessary complexity.  

Design patterns and paradigms are important.  But just because you find one that works well in some situations, doesn't mean it is the best solution in ALL situations.

It depends on the requirements.

# May 9, 2009 11:01 PM

mikesdotnetting said:

I disagree with #1.

The authors of jQuery are heavily into "Unobtrusive Javascript". One of the main tenets of this is that behaviour and structure should not mix, just as style and structure should not mix.

OnClientClick="alert('You Clicked Me!'); will render the behaviour within the structural html tag, which flies in the face of Unobtrusive Javascript.

Quote from jQuery in Action(ISBN: 1933988355):

"Unobtrusive Javascript, along with the legions of the jQuery-savvy, considers any Javascript expressions or statements embedded in the <body> tag of HTML pages, either as attributes of HTML elements (such as onclick) or in script blocks placed within the body of the page, to be incorrect"

# May 10, 2009 4:15 PM

SGWellens said:

Wow, are you a lawyer?  :)

My example was deliberately contrived.

Instead of embedding the alert in the HTML I could just as easily have called a function that called the alert (although some developers would have a problem with the senseless indirection and adding unneeded layers of code)...my point would still be valid.

Don't confuse the separation of behavior and style with connecting behavior to style.

# May 10, 2009 9:19 PM

SGWellens said:

>> Chua Wen Ching said:  
>> In terms of readable is quite subjective.

Yes, but most experienced developers know the difference between good code and bad code.

>> If you have jquery scripts deployed for
>> production, you will prefer it in a compact
>> mode as it is faster.

Yes, that is why there are two versions of jQuery, one for development and one for deployment.

TO: Chua Wen Ching

To keep comments and responses to posts in sync, I had to move your post in here...I hope that is OK.

Steve

# May 10, 2009 9:34 PM

Hadi Hariri said:

Funny how one person's Worst Practice is another's Best Practice.

I disagree on your first 2 points. I number 3, depends how complex your JS is, which using jQuery should be pretty straight-forward. And as someone else commented, if you're going to add comments, make sure you strip them out on build.

# May 11, 2009 1:09 AM

mikesdotnetting said:

If you had used the onclick attribute to point to a function that called the alert, you would still be guilty of worst practice according to Unobtrusive Javascript. Registering event handlers programmatically rather than inline is the recommended way to go - even if it is just one handler for one element that never changes.

>>Don't confuse the separation of behavior and style with connecting behavior to style.

Now you've confused me :o)

# May 11, 2009 3:11 AM

James Brechtel said:

IMHO, #1 can't really be a "Worst Practice" if there are "many times when it IS appropriate to dynamically hookup up event handlers".

Also, whether it's OnClientClick="alert('hello')" or OnClientClick="doSomethingComplex()"  the problem is the same.  More levels of indirection don't change the fact that you're putting behavior in your markup which you'd do good to avoid.

# May 11, 2009 9:56 AM

ewschone said:

Regarding #2, you do realise, that chaining commands can have a different outcome than NOT chaining them ?

# May 11, 2009 10:07 AM

SGWellens said:

>>Registering event handlers programmatically rather
>>than inline is the recommended way to go - even if
>>it is just one handler for one element that never
>>changes.

That is where we disagree.  I recently had a discussion with a proponent of the 3-tier application paradigm.  He insisted it be used ALL the time.  THAT was the disagreement.  When I asked him what the 3 tiers would be in a “Hello World” application he got all confused but gallantly tried to make some up to support his position.

Over-engineering an application is just a bad as under-engineering an application.

# May 11, 2009 10:20 AM

SGWellens said:

>> Regarding #2, you do realise, that chaining
>> commands can have a different outcome than NOT
>> chaining them ?

Really?  The dot operator has a left to right associative precedence.

If what you say was true, putting each logical statement on a separate line  is even more important so you can explicity control the logic.

# May 11, 2009 10:40 AM

SGWellens said:

>> IMHO, #1 can't really be a "Worst Practice" if
>> there are "many times when it IS appropriate to
>> dynamically hookup up event handlers".

It's not the practice itself that is incorrect.  I pointed out that many times it is absolutely the way to go.

The problem is when a good technique is used ALL THE TIME even when it is not appropriate for a situation...that is a worst practice.  Not just for jQuery but for any paradigm

# May 11, 2009 10:53 AM

Hadi Hariri said:

"The problem is when a good technique is used ALL THE TIME even when it is not appropriate for a situation...that is a worst practice.  Not just for jQuery but for any paradigm"

Why is it bad to do that even in the most simple case? And what do you classify as a wrong situation to use that?

# May 11, 2009 12:13 PM

SGWellens said:

"Why is it bad to do that even in the most simple case?

Because you've added extra code and have more moving parts and moved the code that works with the object into a different physical section of the code than the object itself.  (And again, sometimes this is the best way write code...depending on the requirements).

"And what do you classify as a wrong situation to use that?"

That's a really good question.  I wish I could answer it but I would be BS'ing if I tried :)  Maybe that is why software development will always be part art in addition to being an engineering discipline.

# May 11, 2009 12:36 PM

mikesdotnetting said:

>That is where we disagree.

No it's not.  *You* disagree that Unobtrusive Javascript is a good practice.  It's a paradigm you either embrace wholeheartedly, or you don't.  Just like not using tables for page layout, or not using inline CSS. Your comparison with 3 tier architecture completely misses the point.

# May 11, 2009 3:21 PM

LarryB said:

But isn't one of the core tenets of maintainable code following a pattern regardless of need.  I know that in my dev shop if I was to change the way I did something and then expect the support team to support it just because it was easier I would have issues.  The thing that most dev teams/shops attempt to do is to develop a pattern making the code instantly supportable regardless of whether you directly worked on the code or not.  As long as your support team knows the pattern you are following and where to look for things they should be able to support even the simplest code written in the most complex format.  I will agree that sometimes you are building a space ship when all you need is a wagon, but if the pattern helps simplify teaching and support isn't that the best practice in and of it's self?  

# May 11, 2009 3:40 PM

SGWellens said:

> *You* disagree that Unobtrusive Javascript is a good practice.

No I don't.  If you are going to supply my side of the discussion, congratulations you win :).

I disagree that ANY paradigm should be applied 100 percent of the time.

The reason there are so many design patterns is that there are many problems.  You use the one that is appropriate.  You don't use the one that is inappropriate.

I don't think my point is unreasonable or difficult to comprehend.

# May 11, 2009 4:17 PM

SGWellens said:

"But isn't one of the core tenets of maintainable code following a pattern regardless of need."

Yes, great point.  I recently worked on a program where the paradigm was to log the entry and exit points of every function.  Even trivial functions, where nothing could go wrong, had the code.  It was a great paradigm for the situation.

However, the log viewing program wasn't a real-time multi-threaded application.  It did NOT follow the paradigm...there was no need...common sense prevailed.

# May 11, 2009 4:28 PM

mikesdotnetting said:

>Open up the jQuery source code file and look at it.

>You'll see there are more comments than code.  

Deploying that version into production really *would* be worst practice :o)

# May 12, 2009 2:20 AM

SGWellens said:

>>Open up the jQuery source code file and look at it.
>>You'll see there are more comments than code.  

>Deploying that version into production really *would* be
>worst practice :o)

Mike, I agree!

See ya on the forums.

Steve

# May 12, 2009 8:46 AM

Sean Curtis said:

I posted a comment on here the other day but it hasn't appeared - heavily moderated post?

# May 12, 2009 11:44 PM

SGWellens said:

> I posted a comment on here the other day but

> it hasn't appeared - heavily moderated post?

I deleted about 5 comments.

A few repeated points that had already been made and addressed.

A few were confused because they didn't read the text carefully and jumped to conclusions I never made.  

# May 13, 2009 9:44 AM