2

I have 6 list items

$('.favorite-tag-group li').each(function(){
  console.log("hi"); 
});

This code however is displaying "hi" 24 times in console.

The only thing I can think of that might be causing it to bug out is because my list items arent all in the same list.

For example, .favorite-tag-group is a div that always contains a ul. In some cases, that ul will only have 1 li. Sometimes it may have 2.

Here's a sample of what that might look like

div.favorite-tag-group
  ul
    li
    li
    li 
div.favorite-tag-group
  ul
    li
div.favorite-tag-group
  ul
    li
div.favorite-tag-group
  ul
    li
    li

All I'm trying to do is run through .each() li so that I can remove duplicates ;/

Some real html:

<div class="favorite-tag-group">
  <h4>gobty</h4>

  <ul class="resources led-view">
    <li class="clearfix r-tutorial" data-id="22">

    </li>
  </ul>
</div>
<div class="favorite-tag-group">
  <h4>javascript</h4>

  <ul class="resources led-view">
    <li class="clearfix r-tutorial" data-id="24">

    </li>
  </ul>
</div>
<div class="favorite-tag-group">
  <h4>macvim</h4>

  <ul class="resources led-view">
    <li class="clearfix r-tool" data-id="21">

    </li>
  </ul>
</div>  

here is the real function. When i paste the .each() directly into console it works, but inside this function it doesnt work:

 // collapse tags functionality
    $('.collapse-tags').click(function(e){
        e.preventDefault();
        $('.favorites-helpers, .favorite-tag-group h4').slideUp(200, function(){
            var seen = {};
            $('.favorite-tag-group li').each(function(){
                //console.log("hi");
                var currentId = $(this).data('id');
                if (seen[currentId]) {
                    $(this).slideUp(200);
                } else {
                    seen[currentId] = true;
                }
            });
        });
    });
Tallboy
  • 12,847
  • 13
  • 82
  • 173
  • How many times do you expect `hi` to appear given the example in your question? – Andreas Wong Aug 08 '12 at 02:48
  • in the example above, 6 times. – Tallboy Aug 08 '12 at 02:49
  • ah.. yes, 7. my mistake. i expect 7. in actuality, I get the number of list items * the number of groups... so 28 – Tallboy Aug 08 '12 at 02:52
  • @Tallboy Could you post your html? – xdazz Aug 08 '12 at 02:53
  • 2
    Works fine for me: http://jsfiddle.net/ECS2K/ Chrome/Win7 – ahren Aug 08 '12 at 02:56
  • 1 sec, i think it might be the animate function. let me just the paste the whole function above – Tallboy Aug 08 '12 at 02:57
  • I seemed to have FIXED the actual sorting by moving `var seen = {}` right above the .each() rather than right under e.preventDefault(). That somehow fixed the remove duplicate issue, yet it still is logging "hi" to console 30 times. I guess if it works it wroks, although I would really love to know why.... and 2ndly i would love to know why I had to move that variable for it to work – Tallboy Aug 08 '12 at 03:12
  • 2
    @Tallboy, It's because `$('.favorites-helpers, .favorite-tag-group h4')` will be causing multiple elements to `slideUp()`, and therefore the callback gets executed multiple times. Moving `var seen = {}` to inside the callback resets the variable as an empty object in each callback. You'll still iterate over your list items more than once (as seen by multiple `console.log()`s), but you'll slide the same duplicate `li`'s up each time this way. Hope you were able to learn something! – ahren Aug 08 '12 at 03:18
  • wow! That is a SUPERB explanation!! Thanks so much! – Tallboy Aug 08 '12 at 03:31
  • The one thing im still confused about is, why would it not be able to see the scope of `seen` if it were outside the callback? wouldnt variable scope say that it could see it because its outside the function? – Tallboy Aug 08 '12 at 03:42
  • PS put your reply as an answer and ill mark it read – Tallboy Aug 08 '12 at 03:43

2 Answers2

4

As in my comment above... With a bit of further explanation.

It's because $('.favorites-helpers, .favorite-tag-group h4') will be causing multiple elements to slideUp(), and therefore the callback gets executed multiple times. Moving var seen = {} to inside the callback resets the variable as an empty object in each callback. You'll still iterate over your list items more than once (as seen by multiple console.log()s), but you'll slide the same duplicate li's up each time this way.

You asked: "The one thing im still confused about is, why would it not be able to see the scope of seen if it were outside the callback? wouldnt variable scope say that it could see it because its outside the function?"

Yes, you are right - the callback could see seen, but seen was never emptied/reset, and therefore after the second iteration of your callback, all of your li's would have had .slideUp() called on them.

Consider this: because it either slides the duplicate up, or adds the id to seen, on the second callback, .each() runs again, but it's already full of all of your list items ids.

Hope this is clear enough, if not just comment below and I'll try and come up with some examples.

ahren
  • 16,803
  • 5
  • 50
  • 70
  • oh RIGHT! Yesssss *breathes in all the power of understanding* YESSSSS – Tallboy Aug 08 '12 at 04:27
  • 1
    @Tallboy - this also may help you: http://stackoverflow.com/questions/2432267/jquery-calling-a-callback-only-once-after-multiple-animations Here they talk about checking to see whether any of the elements are animated (using `.is(':animated')`), and then executing any further code within a conditional statement. Or this one: http://stackoverflow.com/questions/8793246/jquery-animate-multiple-elements-but-only-fire-callback-once where they use `.promise().done()` – ahren Aug 08 '12 at 05:47
1

Here you are, sir...

$(document).ready(function(){
    $("div.favorite-tag-group>ul").children("li").each(function(index,element){
         //code here
        //refer to element as $(element)
         //to get the id of the element use: $(element).attr("id");
      });
});​

Demo: http://jsfiddle.net/9uz8k/11/

Link:

http://api.jquery.com/each

Andreas Wong
  • 59,630
  • 19
  • 106
  • 123
Dom
  • 38,906
  • 12
  • 52
  • 81
  • `function(element,index){`? or `function(index, element){`? http://jsfiddle.net/9uz8k/9/ – Ram Aug 08 '12 at 03:01
  • 2
    Yes, `api.jquery.com/each` go and see the parameters. `.each( function(index, Element) )` – Ram Aug 08 '12 at 03:05
  • Oh wow! sorry about that! thank you for calling out my stupidity. I made the necessary corrections. – Dom Aug 08 '12 at 03:20
  • 1
    This answer still doesn't address the question at hand. Although the title is a little misleading, it's actually the callback BEFORE `.each()` which is being triggered multiple times. Your answer doesn't help in this case... -1. – ahren Aug 08 '12 at 04:08