ansaurus

Question

jQuery: How to remove this code redundancy?

Answer 1

+2 A:

var list = ['id', 'for']
for (var i in list) {
  var v = list[i];
  clone.find('[' + v + ']').each(function() {
    id = $(this).attr(v);
    ind = id.search(/\d+$/);
    $(this).attr(v, id.substr(0,ind)+id_counter);
  });
}
delete list;

SHiNKiROU 2010-04-11 20:26:14

Uhh ... there's no `foreach` keyword in Javascript. It's a pretty bad idea to write `for` loops with "in" like that, because there might be other things on that array object that the loop would trip over.

Pointy 2010-04-11 20:42:07

... well ok there's `for each` in new Javascript but it means something different and it's really not for arrays anyway.

Pointy 2010-04-11 20:51:20

Answer 2

+3 A:

If all you're trying to do is reduce code, you could...

 function doTheseThings(element) {
    clone.find(element).each(function() {
       var id = $(this).attr('id');
       var ind = id.search(/\d+$/);
       $(this).attr('id', id.substr(0,ind)+id_counter);
    });
 }

 doTheseThings('[id]');
 doTheseThings('[for]');

EVEN BETTER:

I decided to meld my first answer with some of @SHiNKiROU's ideas and add some more Jquery-ish syntax:

var items = ['[id]', '[for]'];
$.each(items, function (index, element) {
    clone.find(element).each(function() {
       var id = $(this).attr('id');
       var ind = id.search(/\d+$/);
       $(this).attr('id', id.substr(0,ind)+id_counter);
    });
});

Byron Sommardahl 2010-04-11 20:29:06

Added `var` on @icktoofay's recommendation.

Byron Sommardahl 2010-04-11 20:31:35

You can't do "array.each()" with jQuery; that's a Prototype thing. You're thinking of `$.each(['id', 'for'], function() { ... })`

Pointy 2010-04-11 20:47:03

On Pointy's recommendation, I fixed the code. I have seen the error of my ways. :)

Byron Sommardahl 2010-04-11 20:48:21

ok! looks fine now

Pointy 2010-04-11 20:53:02

You need to change the parameters in both calls to `attr` as well.

interjay 2010-04-11 20:59:13

Yeah.... shinkirou gave me the idea of using `$.each` too. Works great. I thought about moving it into a function, but I knew there had to be a way without one ;)

Mark 2010-04-11 21:01:45

Answer 3

+1 A:

Well, the redundency is in that the same code applies to the different attributes, so you factor those out:

$.each(['id', 'for'], function (ignr, attr) {
  clone.find('['+attr+']').each(function() {
      val = $(this).attr(attr);
      ind = val.search(/\d+$/);
      $(this).attr(attr, val.substr(0,ind)+id_counter);
  });
});

Then, if you want, you could remove a local variable to shorten it, but I'm not sure this is an improvement:

$.each(['id', 'for'], function (ignr, attr) {
  clone.find('['+attr+']').each(function() {
      val = $(this).attr(attr);
      $(this).attr(attr, val.substr(0,val.search(/\d+$/))+id_counter);
  });
});

ndp 2010-04-11 21:06:41

ansaurus

tags:

views:

answers:

jQuery: How to remove this code redundancy?

related questions