jQuery/javascript optimization

Question

$("#mySelect").change(function() {

  var myVar = $('#MyDiv');

  for (var i=0;i<this.value;i++)
    {
     myVar.clone().appendTo('#formContainer');
    }
});

Any improvements to readability or performance welcome

This kind of question could be posted on codereview.stackexchange.com — JMax
– JMax, Commented Feb 6, 2012 at 15:30
isn't it bad practice to put { on it's own line in javascript? — ianbarker
– ianbarker, Commented Feb 6, 2012 at 15:31
Ugh. Style is a personal thing - it's more important that everyone looking at the file does it the same way than to do it "right". — Grim...
– Grim..., Commented Feb 6, 2012 at 15:40
@ianbarker That's not something I've ever heard before, and really doesn't make any sense to me. I almost always put the { and } characters in my code on their own lines; it just makes it that much more readable to me. — Anthony Grist
– Anthony Grist, Commented Feb 6, 2012 at 15:50

Diodeus - James MacFarlane · Accepted Answer · 2012-02-06 15:29:49Z

4

DOM re-writes are the most expensive. Build your HTML up in memory (or as a string) and insert it in one go AFTER the loop.

answered Feb 6, 2012 at 15:29

Diodeus - James MacFarlane

115k33 gold badges164 silver badges180 bronze badges

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

Comments

ShankarSangoli · Accepted Answer · 2012-02-06 15:39:52Z

3

I agree with @Diodeus, you should go with creating html string and then append it in one go. Try this.

$("#mySelect").change(function() {
  var myVar = $('#MyDiv'),
      html = [],
      count = parseInt(this.value, 10),
      myDivHTML = $('#MyDiv').wrap('<div />').parent().html();
  for (var i = 0; i < count; i++){
     myVar.push(myDivHTML);
  }
  $('#formContainer').append(html.join(''));
});

answered Feb 6, 2012 at 15:39

ShankarSangoli

69.9k13 gold badges96 silver badges124 bronze badges

Comments

devdRew · Accepted Answer · 2012-02-06 15:34:05Z

1

Here you are (that's how I would do):

$("#mySelect").live('change', function() {
    var html = $('<div />').append($('#my-div').clone());
    for (var i = 0; i < this.value; i++) {
        html.append(html.clone());
    }
});

If I got right, your trying to nest clone in div on each iteration... that seems strange to me... Also use live('change', ...); to bind triggers, in this case your code would work not just for loaded at the beginning HTML, but for loaded via AJAX too.

answered Feb 6, 2012 at 15:34

devdRew

4,6013 gold badges27 silver badges34 bronze badges

Collectives™ on Stack Overflow

jQuery/javascript optimization

3 Answers 3

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related