repeating jquery function many times

Question

I need to improve my jquery code where I repeat my function 6 times!!! is there away to do a loop to shorten the code ?

(function( jQuery ){
    jQuery.fn.vesta = function(imgN){
        var imgPath = "http://localhost:8080/mhost/media/magentohost/vesta/vesta"
        var currImg = imgPath + imgN + ".png";
        var targetImg = jQuery('.img-browser img');
        jQuery('.img-browser img').attr('src', currImg);
    }
})( jQuery );

jQuery('.vesta1').on('click', function (e) {
    jQuery('.vesta1').vesta(1);
});
jQuery('.vesta2').on('click', function (e) {
    jQuery('.vesta2').vesta(2);
});
jQuery('.vesta3').on('click', function (e) {
    jQuery('.vesta3').vesta(3);
});
jQuery('.vesta4').on('click', function (e) {
    jQuery('.vesta4').vesta(4);
});
jQuery('.vesta5').on('click', function (e) {
    jQuery('.vesta5').vesta(5);
});
jQuery('.vesta6').on('click', function (e) {
    jQuery('.vesta6').vesta(6);
});

Can you also post your HTML so we can see how its structured. — Rory McCrossan, Jan 26 '15 at 13:14
Was that code snippet meant to do something? What's your HTML? — David Thomas, Jan 26 '15 at 13:14

score 4 · Answer 1 · edited Jan 26 '15 at 13:20

4

You can DRY this up by using a common class, and a data attribute to specify the parameter to send to your vesta function:

<div class="vesta" data-vesta="1">1</div>
<div class="vesta" data-vesta="2">2</div>
<div class="vesta" data-vesta="3">2</div>

Then there is no need to loop at all:

$('.vesta').on('click', function (e) {
    $(this).vesta($(this).data('vesta'));
});

edited Jan 26 '15 at 13:20

Taha Paksu

15,371
2
44
78

answered Jan 26 '15 at 13:17

Rory McCrossan

331,213
40
305
339

meskobalazs · Answer 2 · 2015-01-26T14:13:25.187

2

Just put it into a for loop, and take advantage of the dynamic nature of JavaScript:

for (var i = 1; i <= 6; i++) {
    $('.vesta' + i).on('click', (function (index) {
        return function (e) {
            $('.vesta' + index).vesta(index);
        };
    })(i));
}

edited Jan 26 '15 at 14:13

answered Jan 26 '15 at 13:14

meskobalazs

15,741
2
40
63

1

[JavaScript closure inside loops – simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) – adeneo Jan 26 '15 at 13:20
1

this code won't work. `i` will be bound to the wrong value when the click handler is invoked. – Alnitak Jan 26 '15 at 13:22
Updated my answer with a closure – meskobalazs Jan 26 '15 at 13:39

score 2 · Answer 3 · answered Jan 26 '15 at 13:16

2

Use a common class and a data attribute

jQuery('.vesta').on('click', function (e) {
    var elem = $(this);
    elem.vesta(elem.data("ind"));
});

and the HTML

<div class="vesta vesta1" data-ind="1">

answered Jan 26 '15 at 13:16

epascarello

204,599
20
195
236

score 1 · Answer 4 · answered Jan 26 '15 at 13:15

I suppose you need the this reference along with some hack kind of thing

$('[class*=vespa]').on('click', function(e){
    $(this).vesta(+(this.className.match(/vespa(\d+)/)[1]))
});

Here, we capture elements which have a class that matches at least vespa and then we use some bit of regex to match the digits after vespa and + unary operator changes the String version of numbers into actual numbers.

score 1 · Answer 5 · answered Jan 26 '15 at 13:16

1

It would be quite easy if you can alter the structure of the HTML. You would give all elements the same class, say vesta. But you also give them an attribute, say data-number. For example, like this:

<div class="vesta" data-number="4"></div>

Then, your jQuery code would be as simple as:

$(document).on({
    click: function() {
        var $this = $(this),
            number = +$this.data('number');

        $this.vesta(number);
    }
}, '.vesta');

answered Jan 26 '15 at 13:16

Dennis

14,210
2
34
54

@epascarello thanks, corrected - I forgot how I wanted to call the `on` method while typing my answer! :) – Dennis Jan 26 '15 at 13:20

Ji_in_coding · Answer 6 · 2015-01-26T14:26:59.993

-1

Edit: I was a bit lazy with explaining the code snippet that I have provided an hour ago, but I am modifying my post now in response to the comments.

This code snippet will allow you to apply listeners from '.vesta1' elements to '.vestaN'

[@Variable]

NumberOfClasses - is the positive integer after 'vesta'. Eg: vesta1 ,vesta2, vesta100 ... etc

var NumberOfClasses=6;
for(var i=1;i<=NumberOfClasses;i++){
    var className = '.vesta'+(i+1);
    jQuery(className ).on('click', function (e) {
        $(this).vesta(i);
    });
}

edited Jan 26 '15 at 14:26

answered Jan 26 '15 at 13:15

Ji_in_coding

1,691
1
14
17

2

You should really explain your answer a little. – Liam Jan 26 '15 at 13:17
1

It's just a replacement for those 6 onclick listeners written by the thread opener – Ji_in_coding Jan 26 '15 at 13:20
1

Please explain this in the **answer**, not in a comment. – A.L Jan 26 '15 at 14:07
Why use `i=0;i – A.L Jan 26 '15 at 14:08
It also has the same mistake my solution had. – meskobalazs Jan 26 '15 at 14:13

repeating jquery function many times

6 Answers6