CODEWITHSUNDEEP
javascriptjquery
Zafta
Zafta

Reputation: 667

How can I reduce the copy-paste code? Refactor?

How can I refactor this piece of code so it contains less copy-paste code?

$("#hnv_4").click(function(e){
    manipulate(4);
    e.stopPropagation();
});
$("#hnv_3").click(function(e){
    manipulate(3);
    e.stopPropagation();
});
$("#hnv_2").click(function(e){
    manipulate(2);
    e.stopPropagation();
});
$("#hnv_1").click(function(e){
    manipulate(1);
    e.stopPropagation();
});

Can I use a loop here to minimize the code or maybe some jQuery?

I tried:

for (i = 1; i <= 4; i++) {
 $("#hnv_" + i).click(function (e) {
    alert(i);

});
}

but at the end .. alert shows 5 always

Upvotes: 3

Views: 332

Answers (3)

trevorgrayson
trevorgrayson

Reputation: 1867

I haven't tested this, but this should approximately do what you want to do.

for(var x=1; x<5; x++) { 
  $("#hnv_"+x).click(function(e){
    manipulate( e.target.id.substr(e.target.id.search('_')+1) );    
    e.stopPropagation();
  }) 
}

I'm doing the poor man's solution of scrubbing the "id" number out of the HTML id attribute when I:

e.target.id.substr(e.target.id.search('_')+1)

But if you are using jQuery, you may have the ".data" method where you could store the id in, and then not have to do that scrubbing hack. e.g.

for(var x=1; x<5; x++) { 
  $("#hnv_"+x).data("id",x).click(function(e){
    manipulate( $(e.target).data("id") );    
    e.stopPropagation();
  }) 
}

Not tested! feel free to correct me.

Upvotes: 0

Barmar
Barmar

Reputation: 782315

Change your HTML to this:

<div class="hnv" data-hnv="1">...</div>
<div class="hnv" data-hnv="2">...</div>
and so on

Then change the jQuery to:

$(".hnv").click(function(e) {
    manipulate($(this).data("hnv"));
    e.stopPropagation();
}

If you want to do it with a for loop, you have to capture the index in closure, using an immediately-executing function:

for (var i = 1; i <= 4; i++)
{
    (function (i) {
        $("#hnv_" + i).click(function(e){
            manipulate(i);
            e.stopPropagation();
        })
    })(i);
}

See JavaScript closure inside loops – simple practical example for the explanation of why your loop doesn't work, and the extra function is needed.

Upvotes: 9

tymeJV
tymeJV

Reputation: 104795

Yes,

$("[id^='hnv_']").click(function(e) {
    var number = Number(this.id.split("_")[1]);
    manipulate(number);
    e.stopPropagation();
});

Upvotes: 12

Related Questions