-1

Edit (torazaburo was correct):

For future reference, and since there appears to be a lot of confusion about closures by many who commented, here are some notes.

You can write the code as I did below, in fact it was correct. The problem was, I simplified my specific case too much. It was not the way I wrote the closure. My problem was the DOM element was a table cell td, which had not been appended to the row (newRow.appendChild(newCol)) at that specific point in time when I was adding the hover. Consequently the cell was not in the DOM at that point in time. Consequently jQuery did not know it existed.

The original question:

My code:

var i, strId;

for(i=0;i<10;i++){
    // strId is the id of the element without the # needed for jquery
    strId = "idString" + i.toString();

    // Some stuff ...

    // It is done this way so the value of i is retained in the functions in the hover
    (function (i, strId) {
        $("#" + strId).hover(function () { MyFunctionIn(i);}, function () { MyFunctionOut(i); });
    })(i, strId);
}

This does not work as written.

The value of i is being retained, as I have tested this with just $("#idString0") in place of $("#" + strId) and this works fine.

But when I put the variable strId back into the hover it fails.

Anyone know what I am doing wrong?

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Rewind
  • 2,554
  • 3
  • 30
  • 56
  • Just use `.on()`; this logic seems crazy. – Cory Silva Jul 01 '16 at 19:01
  • Thanks for the reply Cory, could you explain more? – Rewind Jul 01 '16 at 19:02
  • Well you are using `i` to build a HTML id attribute. Then you are trying to attach a hover function to each item with a closure to the `i` variable. You could simply pull this off the element attributes. More importantl, though, you should just use the `.on()` method. – Cory Silva Jul 01 '16 at 19:06
  • 1
    This falls into the category of laying on a bed of nails and then wondering why there are little holes in your back that hurt. What are you trying to do on hover, anyway, and is it something that could not be done with CSS? Constructing IDs as strings and addressing the DOM using them is a horrible anti-pattern, that you should replace with logic which simply works with elements themselves. If you would move on to ES6, you could completely stop worrying about this, even if you wanted to use the same kind of logic, by using `for (let i)`. Anyway, if you insist on jQuery please tag the question. –  Jul 01 '16 at 19:10
  • Thanks again for the reply. I do not understand your sentence 'You could simply pull this off the element attributes'. I also do not understand why you do not like 'hover' and want to use 'on' instead. – Rewind Jul 01 '16 at 19:10
  • It works on [JSFiddle](https://jsfiddle.net/lhz213/qkntfej2/) – Haizhou Liu Jul 01 '16 at 19:14
  • 2
    Anyway, there is no reason why what you have written, however ill-advised, should not work, so the problem must lie elsewhere. I assume you tried to debug this, right? So you put an breakpoint inside `MyFunctionIn` and verified the value of `i` that was getting passed in, right? And you added a `console.log` inside the anonymous function to see what the values of `i` and `strId` were, right? When you say "does not work as written", in precisely way does it not work? What are the symptoms? We are not mind-readers. –  Jul 01 '16 at 19:21
  • If I build the string inside the closure, then hover is not attached to the element. That is the problem. SoWeLie seems to think this is the problem (see his answer). I am reading his mozilla link at this time. – Rewind Jul 01 '16 at 19:26
  • As I said, DEBUG YOUR PROGRAM. Add breakpoints. Add console.logs. Review your code--for instance, if you misspelled the name `strId` in the parameter list to the anonymous function it will use the `strId` from the outer scope. Provide us with the **exact** code you are actually using. There is an extremely high probability that you will find that your problem lies elsewhere. –  Jul 01 '16 at 19:30
  • Thanks for your input toazaburo, but I am debugging, I have checked spelling. I am reading the mozilla article. All I can say at this stage is the closure does not like the second parameter when I am using it for the $() bit of the code. – Rewind Jul 01 '16 at 19:35
  • All I can say at the moment is I think SoWeLie is right, but I do not know why. – Rewind Jul 01 '16 at 19:36
  • Your code works fine as is: https://jsfiddle.net/ctoe09p8/, or here with `strId` too: https://jsfiddle.net/ctoe09p8/1/ - if you are having problems, perhaps it is in the implementation of the `MyFunctionIn()` and `MyFunctionOut()`. – nnnnnn Jul 01 '16 at 19:36
  • 1
    Here is another fiddle showing the code you have provided working perfectly: https://jsfiddle.net/ym7md1me/. How can SoWeLie be right when your code already works before his changes? –  Jul 01 '16 at 19:42
  • If you are debugging, then please provide a screenshot or console dump of a log of the value of `strId` within the closure. –  Jul 01 '16 at 19:46
  • torazaburo - you are right. It was a table cell td, and I had not added it to the row, which meant I had not added it to the DOM, when I added the hover. In other words I was trying to add a hover to something the DOM and jquery could not see at that point in time. – Rewind Jul 01 '16 at 20:18
  • If you want to put an answer, I will tick that, otherwise I will delete the question. – Rewind Jul 01 '16 at 20:19
  • @Rewind Well, my comments do not constitute an "answer", they merely postulated the existence of some other factor for your code not working, which you have now identified, congratulations. You can delete your question, if you want, or you could wait for enough people to vote to close it as a typo and it will be closed that way. –  Jul 02 '16 at 05:25
  • Sorry about my answer. I quickly assumed you had made the common mistake of misunderstanding JavaScript scoping. – SoWeLie Jul 02 '16 at 13:01

1 Answers1

0

This is not a direct answer

You are experiencing problems with function scopes. Despite the scope problem, I think you may have an architectural anti-pattern forming. In one of your comments you mentioned that you cannot use on because "different elements have different things they do when I am hovering over them". This can simply be handled by a strategy pattern.

Reference this Plunker and the Jquery documentation

HTML

<div data-type="a" data-id="1" class="hover">A Type Element</div>
<div data-type="a" data-id="2" class="hover">A Type Element</div>
<div data-type="b" data-id="1" class="hover">B Type Element</div>

Javascript

// Define a service to help with hovering behaviours
function hoverService() {
  var eventMap = {
    mouseenter: 'On',
    mouseleave: 'Off'
  };
  var service = {
    hoverStrategy: hoverStrategy,
    aOn: aOn,
    aOff: aOff,
    bOn: bOn,
    bOff: bOff
  };
  return service;
  // Use this as a proxy to determine the right action to take
  function hoverStrategy(event) {
    if (!event || !event.currentTarget) {
      return;
    }
    var $element = $(event.currentTarget);
    var id = $element.data('id');
    var elementType = $element.data('type');
    var eventType = eventMap[event.type];
    var strat = elementType + eventType;
    return service[strat] ? service[strat](id, $element) : false;
  }

  function aOn(id, $element) {
    console.log(id, $element, 'aOn');
  }

  function aOff(id, $element) {
    console.log(id, $element, 'aOff');
  }

  function bOn(id, $element) {
    console.log(id, $element, 'bOn');
  }

  function bOff(id, $element) {
    console.log(id, $element, 'bOff');
  }

}

$(document).ready(function() {
  var service = hoverService();
  $('.hover').on('mouseenter', service.hoverStrategy);
  $('.hover').on('mouseleave', service.hoverStrategy);
});
Community
  • 1
  • 1
Cory Silva
  • 2,761
  • 17
  • 23
  • You are missing the point of his question, he's running into the classic scope inside of loop problem when dealing with callback functions. See this question / answer here: http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example – SoWeLie Jul 01 '16 at 19:24
  • No I am not missing the point. Sure he does not understand scope, but he definitely does not understand how to structure this behavior. Furthermore making all these scope closures is not performant. – Cory Silva Jul 01 '16 at 19:28
  • I cannot use 'on' as different elements have different things they do when I am hovering over them. – Rewind Jul 01 '16 at 19:31
  • 1
    Why would this change be necessary when the code shown in the question already works with the `.hover()` function in the loop: https://jsfiddle.net/ctoe09p8/1/ You haven't explained what was wrong or why this would fix it, and you've combined the in and out handlers into a single function without explaining how to use `event` to tell which event happened. – nnnnnn Jul 01 '16 at 19:40
  • 1
    Your solution might work and might be more elegant, but it would seem better to answer the OP's specific question about why his code doesn't work (or why it should work) and "how to retain the value of an iteration", rather than proposing something entirely new. –  Jul 01 '16 at 19:56
  • @torazaburo You guys are right; I am not answering his direct question. I have edited my "answer" to reflect – Cory Silva Jul 01 '16 at 21:37