jQuery function optimization

Question

Is there a way to make this faster?

This function manages some dynamic form elements.

 function HideAllDivs(parentDiv) {
        var divs = $('#' + parentDiv + ' div'), divsLength = divs.length;   
        for (var a = 0; a < divsLength; a++) {
            var obj = $('#' + divs[a].id);
            obj.find('input[type=radio]:checked').removeProp('checked');
            obj.find('input[type=checkbox]:checked').removeProp('checked');
            obj.find('input[type=text]').val('');
            obj.find('select').val('');
            obj.find('input[type=radio]').each(function () { $(this).removeClass('selectedCtrl');   });
            obj.hide(1);                           
        }    
    }

Thanks, David

Isn't var obj = $('#' + divs[a].id); the same as saying var obj = $(divs[a]); ? — techfoobar
– techfoobar, Commented Oct 14, 2013 at 14:07
I think the best way to optimize this is to convert it to pure JS — Mike H.
– Mike H., Commented Oct 14, 2013 at 14:10
Question. in the variable divs, you have $('#' + parentDiv + ' div') , does this mean you have multiple divs with the same ID on the page and you are trying to iterate through them? That's what it looks like and it's invalid html, ID's should always be unique. — Daniel Ward
– Daniel Ward, Commented Oct 14, 2013 at 14:12
@DanielWard It looks to me like he doesn't have divs with the same ID, which is why he has a variable in there — Reinstate Monica Cellio
– Reinstate Monica Cellio, Commented Oct 14, 2013 at 14:12

techfoobar · Accepted Answer · 2013-10-14 14:27:49Z

4

You can speed it up (slightly) by using..

function HideAllDivs(parentDiv) {
    $('#' + parentDiv + ' div')
        .hide()
        .find('input, select')
            .removeProp(':checked') // will be ignored where not applicable
            .removeClass('selectedCtrl') // will be ignored where not applicable
            .filter('input[type="text"], select').val('');
}

edited Oct 14, 2013 at 14:27

answered Oct 14, 2013 at 14:22

techfoobar

66.8k14 gold badges117 silver badges138 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

simey.me Over a year ago

I think it's worth noting the original poster's code and comment made it appear that there were restraints... ie: If you have a input[type=text].selectedCtrl that you do not want to remove the class from, techfoobar's code fails... but his is extremely fast if you do not have to worry about such constraints :) added it to: jsperf.com/hiding-divs-19362135

Shrink Over a year ago

Only the radio buttons have the 'selectedCtrl' class... ;-)

Arun P Johny · Accepted Answer · 2013-10-14 14:19:32Z

1

There is no need to use a loop and you can improve the 2 operations using radio buttons

function HideAllDivs(parentDiv) {
    var divs = $('#' + parentDiv + ' div');

    var radios = divs.find('input[type=radio]')
    radios.filter(':checked').removeProp('checked');
    radios.find('.selectedCtrl').removeClass('selectedCtrl');

    divs.find('input[type=checkbox]:checked').removeProp('checked');
    divs.find('input[type=text], select').val('');
    divs.hide(1);
}

edited Oct 14, 2013 at 14:19

answered Oct 14, 2013 at 14:09

Arun P Johny

389k68 gold badges532 silver badges532 bronze badges

5 Comments

Mike H. Over a year ago

Not that I doubt you, but could you elaborate on how this makes the function faster? I would think finding each unique id and iterating through the results seems more efficient than having to run .find() on the set of divs selected from jQ...but that just may be my ignorance on how .find() works :P

jantimon Over a year ago

Why do you filter for ':checked' if you remove checked anyway

Arun P Johny Over a year ago

when you say this.find(), there is a iteration happening from jQuery... so in OP's case the iteration is happening 2n times instead of n

Mike H. Over a year ago

I see, so couldn't he essentially remove the .find()'s and leave his original iteration, using selectors instead?

Arun P Johny Over a year ago

@jantimon that one need to be profiled.... whether trying to remove a non existent property is costlier or filtering it

simey.me · Accepted Answer · 2013-10-14 15:30:09Z

0

I thought this should be slightly faster...

function HideAllDivs(parentDiv) {

  var $divs = $('#' + parentDiv + ' div');

  $divs
    .find('input[type=text],select').val('').end()
    .find(':checked').removeProp('checked').end()
    .find('.selectedCtrl[type=radio]').removeClass('selectedCtrl').end()
    .hide(1);
}

Performance tests below
* Edit: Fail, I didn't actually run the tests -_-; fixed.*
http://jsperf.com/hiding-divs-19362135

edited Oct 14, 2013 at 15:30

answered Oct 14, 2013 at 14:25

simey.me

2,2071 gold badge21 silver badges22 bronze badges

2 Comments

simey.me Over a year ago

Shrink if you're looking for a succinct code, mine is fairly pretty and readable, but check the tests... what I thought was fast is actually slow... even as a fairly experienced jQuery dev I'm struggling to understand why, but JSPerf doesn't lie.

simey.me Over a year ago

Again, I failed in the test, I was running the code from the point of view of our answers, not taking into account your original code was using the ids , it turns out mine is fairly fast, but there's a faster one. It's up to which you chose... there's trade off between readability and speed

Collectives™ on Stack Overflow

jQuery function optimization

3 Answers 3

2 Comments

5 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

5 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related