jQuery/javascript re-structuring issues

Question

Issue:

I just started using javascript/jQuery with little experience. I had working code, but after a little bit of restructuring, everything died.

Original Code:

// Floating NavBar - Side
var names = ['#floatMenu','#header'];

for (var i = 0; i < names.length; i++){
    floatObj(names[i])
}

function floatObj(name)
{
    var menuYloc = null;
    $(document).ready(function(){
        menuYloc = parseInt($(name).css('top').substring(0,$(name).css('top').indexOf('px')));
        $(window).scroll(function(){
            var offset = menuYloc + $(document).scrollTop() + 'px';
            $(name).animate({top:offset},{duration:0,queue:false});
        });
    });
};

Re-structured Code

$(document).ready(function(){
    floatObj();
});

function floatObj()
{
    var names = ['#floatMenu','#header'];
    var menuYloc = null;
    for (var i = 0; i < names.length; i++){
        menuYloc = parseInt($(names[i]).css('top').substring(0,$(names[i]).css('top').indexOf('px')));
        $(window).scroll(function(){
            var offset = menuYloc + $(document).scrollTop() + 'px';
            $(names[i]).animate({top:offset},{duration:0,queue:false});
        });
    };
};

Question:

I was wondering if someone could point out why restructuring the code this way doesn't work? I was also wondering if there was a method of debugging javascript without any additional add-ons? (It would also be helpful if glaring mistakes are pointed out.)

Reason why.

The reason I want to re-structure my code this way is because I have some other functions I would like to run at load up. I figured I could throw all the functions into the $(document).ready(function(){}) bit. If there's actually a proper way of doing this, please enlighten me ><.

Any help would be greatly appreciated! Thanks.

see http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example — Musa, Aug 12 '13 at 18:58

score 2 · Accepted Answer · answered Aug 12 '13 at 18:56

2

The problem is that i, when the callback you pass to scroll is called, has the value it has at end of loop (names.length).

In your first code, the floatObj function created a scope storing the value of names[i].

Most solutions involve calling a function in the for loop. If you don't want to call a named external function, you may do this :

for (var i = 0; i < names.length; i++){
    (function(i){ // this stores i in the scope of this function
      menuYloc = parseInt($(names[i]).css('top').substring(0,$(names[i]).css('top').indexOf('px')));
      $(window).scroll(function(){
        var offset = menuYloc + $(document).scrollTop() + 'px';
        $(names[i]).animate({top:offset},{duration:0,queue:false});
      });
    })(i);
};

answered Aug 12 '13 at 18:56

Denys Séguret

372,613
87
782
758

@sihrc : use this solution, it's cleaner than mine. – m_vdbeek Aug 12 '13 at 18:58
Thanks for both of your answers! Now, I understand that this is a scope issue, but I'm not clear on why the 'i's aren't used before its value is changed? Is it just the way javascript runs things? (I'm more used to coding in python if that makes explaining easier). – sihrc Aug 12 '13 at 19:04
2

The function you pass to scroll isn't immediately called. When it's called, `i` is used with the value it has. The other point you may miss is that the scope of a non global variable is the function in which it is defined, that's why solutions involve a function. – Denys Séguret Aug 12 '13 at 19:07
Ah, I think I understand now. I was missing the point that there was another function() passed into scroll. Thanks! – sihrc Aug 12 '13 at 19:10

score 1 · Answer 2 · answered Aug 12 '13 at 18:56

The issue is most likely closures here, try this :

$(document).ready(function(){
    floatObj();
});

function myClosure(menuYloc, name, i) {
  $(window).scroll(function(){
      var offset = menuYloc + $(document).scrollTop() + 'px';
      $(names[i]).animate({top:offset},{duration:0,queue:false});
  });
}

function floatObj()
{
    var names = ['#floatMenu','#header'];
    var menuYloc = null;
    for (var i = 0; i < names.length; i++){
        menuYloc = parseInt($(names[i]).css('top').substring(0,$(names[i]).css('top').indexOf('px')));
        myClosure(menuYloc, name, i);
    };
};

What most likely happens, is that the value of i is constant and thus your other name,#header`is never bound to your animation.