0

I already know how to loop thanks to closure like this :

for (var i = first; i <= last; i++) {
    document.getElementById(more + i).onmouseover = (function(arg1) {
        return function() {
            document.getElementById(arg1).style.display = "block";
        }
    })(i);
}

But I'd like now to do a look inside the loop. I've got elements of the DOM which must be dynamically attached to other elements. The elements are of the type: '1text1', '1text2', '1text3',... and '2text1', '2text2', '2text3', '3text4'... I've tried something like this:

var text;
var ktext;
for (var k = 1; k <= last_number; k++) {
    for (var i = first; i <= last; i++) {
        ktext = k + text;
        document.getElementById(ktext + i).onmouseover = (function(arg1, arg2) {
            return function() {
                document.getElementById(arg1 + arg2).style.display = "block";
            }
        })(ktext, i);
    }
}

But the loop is partially lost. If I put alert(ktext + i) just at the beginning of the var i loop, I see: 1text1, 1text2, 1text3, 1text4 then 2text1 (as expected), but not 2text2, 2text3... as if the second pass of the var k loop stops too early.

2
  • 1
    You're binding event handlers in loops? Have you considered event delegation? Dec 29, 2012 at 20:41
  • You're using the variable text before it was initialized.
    – David G
    Dec 29, 2012 at 20:42

1 Answer 1

2

Your use of closures is unnecessary:

for( k=1; k<=last_number; k++) {
  for( i=first; i<=last; i++) {
    document.getElementById(k+text+i).onmouseover = function() {
      this.style.display = "block";
    }
  }
}

Anyway, your loop may be stopping early if one of the elements doesn't exist on the page. You should check for the element's existence before attempting to assign to it.

2
  • My bad, I wanted to simplify to post my code here and I cut some useful parts: like : document.getElementById(arg1+arg2+more).style.display="block" The two elements are different
    – ThePhi
    Dec 29, 2012 at 20:55
  • Well, in any case you can get the ID of the element with this.id, so maybe you can use that instead of having to create that many closures. You might even be able to delegate the event so you remove the loops entirely and just do: document.body.onclick = function(e) {e = e || window.event; var t = e.srcElement || e.target; var id = t.id; /* now do stuff based on the ID */}; Dec 29, 2012 at 20:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.