CODEWITHSUNDEEP
jqueryxmlajaxperformance
Rachel
Rachel

Reputation: 235

How to do a for loop to make this code more efficient?

I'm a designer that dabbles in jQuery and it's a really steep learning curve for me, I know that it is very powerful but I struggle to avoid millions of lines of code and still seem to be hard coding which I'm pretty sure can be reduced and softened. I was wondering if someone could please advise on the below code example? Should i be combining it in a loop, if so could someone point me in the right direction? Does it need to be hard coded and every selector added?

$(xml).find('Data').each(function(){

  $('#saved').val($(this).find('saved').text());
  $('.jobrole').text($(this).find('jobrole').text());
  $('#psu1').val($(this).find('psu1').text());    
  $('#psu2').val($(this).find('psu2').text());
  $('#psu3').val($(this).find('psu3').text());
  $('#tsu1').val($(this).find('tsu1').text());
  $('#tsu2').val($(this).find('tsu2').text());
  $('#tsu3').val($(this).find('tsu3').text());
  $('#pm1').val($(this).find('pm1').text());
  $('#pm3').val($(this).find('pm3').text());
  $('#tm1').val($(this).find('tm1').text());
  $('#tm3').val($(this).find('tm3').text());
});

Any advice would be hugely appreciated. I have started to research MVC frameworks, would this help/ is this the way forward?

Kindest regards Rachel

Upvotes: 0

Views: 76

Answers (3)

Stuart Burrows
Stuart Burrows

Reputation: 10814

I'd probably do something along these lines:

$(xml).find('data').children().each(function(){
    var $self = $(this);
    var nodeName = this.nodeName.toLowerCase();
    var identifier = nodeName == 'jobrole' ? '.' : '#';

    $(identifier + nodeName ).val($self.text());
});

Here's the fiddle (modded from @dystroy's original): http://jsfiddle.net/lnrb0b/xeNdQ/ ​You can see I've assumed lowercase class / id values. I've also assumed you won't have much duplication in the xml.

Left two <data> elements in the generated xml just in case you do have multiple.

Upvotes: 0

Denys S&#233;guret
Denys S&#233;guret

Reputation: 382384

Here's the basic idea :

var xmlelement = $(this);
$('[id^="psu"],[id^="tsu"],[id^="pm"],[id^="tm"]').each(function(){
   $(this).val(xmlelement.find($(this).attr('id')).text());
});

Note also that jQuery needs lowercase tags, so you need to use this :

​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​$(xml).find('data').each(function(){

The complete code might be (depending on your inputs and your xml) 

$(xml).find('data').each(function(){
    var xmlelement = $(this);
    $('.jobrole').text($(this).find('jobrole').text());
    $('input').each(function(){
       var text = xmlelement.find($(this).attr('id')).text();
       if (text.trim()) $(this).val(text);
    });
});

Demonstration

​Note that in this code I had to guard against Data elements not having some elements. Without this test (if (text.trim())), only the last Data would affect your inputs).

Upvotes: 1

Phil H
Phil H

Reputation: 20151

Basic reduction of repeated lines:

$(xml).find('Data').each(function(){
  var ids = ['saved', 'psu1', 'psu2', 'psu3', 'tsu1', 'tsu2', 'tsu3',
             'pm1', 'pm3', 'tm1', 'tm3'];

  $('.jobrole').text($(this).find('jobrole').text());

  for(var idx = 0; idx < ids.length; ++idx) {
      $('#'+ids[idx]).val($(this).find(ids[idx]).text());
  }
});

Then perhaps construct the ids array elsewhere and pass it in, since I guess these are coming from some other piece of code.

Upvotes: 0

Related Questions