ansaurus

Question

Answer 1

+3 A:

One thing that jumped out at me...

To speed things up use:

$('input:checkbox[name=checkbox]')

instead of

$(':checkbox[name=checkbox]')

This is because the first version look for checkboxes only among the input elements, the second version looks for checkboxes over all elements.

Take a look at this speed comparison Nick Craver created to demonstrate the above for this answer.

Do this wherever possible, so use $('input:checked').

Also, there are several jQuery objects you create multiple times.

For example you use, $(':checkbox[name=checkbox]') many times. Each time you recreate the same jQuery object, so do:

var $nameCheckbox = $('input:checkbox[name=checkbox]');
$nameCheckbox.click(function() {
    ...
       $nameCheckbox.not(':checked').attr('disabled', true);
    } else {
       $nameCheckbox.filter(':disabled').attr('disabled', false);
    ...

Finally semantically, it makes more sense to remove the inline javascript, so replace:

<body onLoad="clearForms()" onUnload="clearForms()">

with

<body>

and put clearForm() inside the jQuery doc ready. The jQuery doc ready will fire before window.onload since onload has to wait for all images, etc to load. jQuery doc ready fires when the DOM is ready. You can also make use of window.onunload, but I don't quite understand why that's necessary.

$(document).ready(function() {
    clearForms();

Peter Ajtai 2010-10-04 22:09:02

Thank you for looking at this, it is much appreciated.

James 2010-10-05 00:10:17

@James - You're welcome ;) I added one more thing about removing the inline JS and add `clearForms()` into doc ready which should fire a little before `onload` if you have any images and such on the page.

Peter Ajtai 2010-10-05 01:24:45

So for example would something like this be correct? $('input:checkbox[name=checkbox]').not('input:checked').attr('disabled', true);

James 2010-10-05 01:27:20

@James - In that case,`$('input:checkbox[name=checkbox]')` is a set of all `input` checkboxes with the name `checkbox`, so it is redundant to write `.not('input:checked')`. Instead write `.not(':checked')`......... In fact that's the example I wrote in my code above: `$nameCheckbox.not(':checked').attr('disabled', true);` which because of what `$nameCheckbox` is defined as is: `$('input:checkbox[name=checkbox]').not(':checked').attr('disabled', true);`

Peter Ajtai 2010-10-05 01:35:22

Gotcha, Thanks for your help :) Thats exactly what I was wondering, whether or not 'input:checked' was necessary. Im going to implement the changes to the clearforms funtion as well. Can I post the code once I think it meets your advice and have you take one last look at it?

James 2010-10-05 01:44:28

I think I have edited to meet your advice, I had to add to jfiddle because the code is too long for a message. here is the link: http://jsfiddle.net/xSsrC/2/

James 2010-10-05 01:56:52

@James - That looks pretty good. I notice you didn't include a [ `window.onunload = clearForms;` ](https://developer.mozilla.org/en/DOM/window.onunload) - But I'm not sure what function that served any way.

Peter Ajtai 2010-10-05 02:06:47

I skimmed right over that section about window.onunload, was trying to modify quickly to get advice on mods. It seems to be clearing just fine though. When I first started, I had a problem with FireFox holding onto disabled checkboxes when reloading page, but it seems to be cleared up. Adding this code: $(':checkbox[name=checkbox]:disabled').attr('disabled', false);, to ClearForms() is the only thing that worked to clear disabled checkboxes onload, resetting the form wouldnt quite do the trick.

James 2010-10-05 02:15:06

ansaurus

tags:

views:

answers:

Can this code be optimized?

related questions