Condensing identical functions

Question

I am trying to write a bit of JavaScript to help condense this jQuery set of functions (numbered 1 through 7) ...

    $('.project-1 img').hover(function() {
      $('#id1').slideToggle();
    });
    $('.project-2 img').hover(function() {
      $('#id2').slideToggle();
    });
    ...
    $('.project-7 img').hover(function() {
      $('#id7').slideToggle();
    });

I tried this following bit of code, but it only targets #id7 with slideToggle...

    for (i = 1; i <= 7; i++) {
      var projectNum = '.project-' + i + ' img';
      var idNum = '#id' + i;
      $(projectNum).hover(function() {
        $(idNum).slideToggle();
      });
    };

How do I condense the code into something cleaner? Thanks!

This may be okay for Code Review if a) the code works b) the code isn't hypothetical or incomplete in any way — Quill, Jul 12 '16 at 22:09
You have the number you need in the class attribute, so just use a regex to extract it. `this.className.match(/project-(\d+)/)[1]` So then all you need a common class to use to select the group of elements in the first place. — , Jul 12 '16 at 22:10
http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example — Evan Trimboli, Jul 12 '16 at 22:12
You can avoid the use of an ugly loop/closure completely here if you use DRY principles - ie. common classes, data attributes for unique data and a single event handler — Rory McCrossan, Jul 12 '16 at 22:12
...while the duplicate is indeed related to the OP's attempt, I don't think that's really the core of the question. The attempted solution isn't a great approach here. But then at its core it is off-topic. — , Jul 12 '16 at 22:14
Part of your problem is that you are using what appears to be a unique identifier for the class attribute, eg `project-1, project-2`, etc, when classes should be more generic. If each of these elements had a common set of classes, like `class='project img'`, then you could use a selector that referenced all of them, like `$('project img').hover`. — Anthony, Jul 12 '16 at 22:16

score 4 · Answer 1 · answered Jul 12 '16 at 22:13

4

While a loop would work (if you fix the closure problem) a much better solution would be to instead use DRY principles to attach a single event handler which would work for all img elements and avoid the need for any loops or closures at all. Try this:

<div class="project">
    <img src="foo.jpg" data-target="#id1" />
</div>
<div class="project">
    <img src="bar.jpg" data-target="#id2" />
</div>
<!-- and so on... -->

<div id="id1">Lorem</div>
<div id="id2">Ipsum</div>
<!-- and so on... -->

$('.project img').hover(function() {
    var target = $(this).data('target');
    $(target).slideToggle();
});

answered Jul 12 '16 at 22:13

Rory McCrossan

331,213
40
305
339

Nice. Much better to address the ultimate question than patching a sub-optimal attempt. – Jul 12 '16 at 22:17
Neat! I didn't even know this was a possibility. – ms_nitrogen Jul 12 '16 at 22:23

score 0 · Accepted Answer · answered Jul 12 '16 at 22:10

0

Commenters above have correctly diagnosed the problem. Here's a fix:

for (i = 1; i <= 7; i++) {
  (function (i) {
    $('.project-' + i + ' img').hover(function () {
      $('#id' + i).slideToggle();
    });
  })(i);
};

answered Jul 12 '16 at 22:10

user94559

59,196
6
103
103

Condensing identical functions

2 Answers2