Javascript scope -- var vs global

Question

For currentUnknownBox, if I use "var" the expected functionality does not work as expected (currentUnknownBox becomes the first element clicked). If I remove the var, it works as expected. I am assuming this has something to do with global scope. Could someone explain this to me?

jQuery(".box.unknown").live('click',function()
{
    var currentUnknownBox = this;

    //if we are NOT on mobile, use jQuery UI dialog
    if (!Drupal.settings.is_mobile)
    {
        jQuery("#letter-input-dialog").dialog();

        jQuery('#letter_input_form').submit(function()
        {
            var letter = jQuery("#letter_input").val();
            jQuery("#letter-input-dialog").dialog('close');
            jQuery("#letter_input").val('');
            that.validateAndSaveLetter(currentUnknownBox, letter);
            //Do not let the form actually submit
            return false;
        });
    }
    else
    {
        var letter = prompt('Please enter a letter to use in your guess');
        that.validateAndSaveLetter(that.currentUnknownBox, letter);
    }
});

EDIT: The problem is I am re-declaring my submit function every time.

`currentUnknownBox` seems to be fine, but what is `that`? And why is there also a `currentUnknownBox` property on the `that` variable (expected in the else clause)? — Bergi, Apr 18 '13 at 16:48
unrelated to your problem, but in JS curly-brace placement matters and should probably not be on their own lines: http://stackoverflow.com/questions/3641519/why-results-varies-upon-placement-of-curly-braces-in-javascript-code — Zach Lysobey, Apr 18 '13 at 17:00
@ZachL: That's only for object literals. Where you put block braces is personal coding style, and (although I don't like) [Allman style](http://en.wikipedia.org/wiki/Indent_style#Allman_style) is fine — Bergi, Apr 18 '13 at 17:25
@Bergi true, but it's important to be consistent, so even a minor edge case (which this is not) is enough reason to not use Allman style IMO. — Zach Lysobey, Apr 18 '13 at 17:30
@ZachL: I don't see inconsistencies in the OPs code. Or did you mean the object literal? That's not an edge case, but a completely different thing. — Bergi, Apr 18 '13 at 17:34
Sorry for the micommunication. I was trying to be terse to avoid a lengthy discussion of the topic of brace placement in this unrelated question. OPs code does *not* have inconsistancies, but if he were to declare an object literal (using egyptian brakets to avoid semi-colon insertion problems) then his brace placement *would* be inconsistant. If another programmer not intimately familiar with JS comes along, and copies his Allman style while using an object literal, than very likely he would have a hard time debugging resulting problems. More here: http://stackoverflow.com/questions/11247328 — Zach Lysobey, Apr 18 '13 at 17:43

score 3 · Accepted Answer · answered Apr 18 '13 at 16:55

The problem is that every time one of these is clicked, you're adding a new submit event handler to your form. But the first one will always fire first. When you don't declare the var, you're overwriting the variable that that first handler is looking at. But the mistake is adding a new handler every time. I would do it like so:

var currentUnknownBox;
jQuery('#letter_input_form').submit(function()
    {
        var letter = jQuery("#letter_input").val();
        jQuery("#letter-input-dialog").dialog('close');
        jQuery("#letter_input").val('');
        that.validateAndSaveLetter(currentUnknownBox, letter);
        //Do not let the form actually submit
        return false;
    });
jQuery("#letter-input-dialog").dialog({autoOpen: false});

jQuery(".box.unknown").live('click',function(){
    currentUnknownBox = this;

    //if we are NOT on mobile, use jQuery UI dialog
    if (!Drupal.settings.is_mobile)
     {
       jQuery("#letter-input-dialog").dialog('open');
      } else {
        var letter = prompt('Please enter a letter to use in your guess');
       that.validateAndSaveLetter(currentUnknownBox, letter);
      }   
});

Incidentally, .live is deprecated. You should use .on instead.

score 1 · Answer 2 · answered Apr 18 '13 at 16:54

Whenever you use var you are declaring the scope of a variable. If you omit it then JavaScript assumes the most compatible option which is global. The technical term is "hoisting". And to be more accurate, JavaScript has what is called functional scoping so even if you declare a variable inside a for loop, JavaScript is "hoisting" it to the top of the closest function.

score 0 · Answer 3 · answered Apr 18 '13 at 16:48

0

You forgot to declare the var 'that'. I think you need to do this before 'if' statement

var that = this;

answered Apr 18 '13 at 16:48

Marcelo Rodovalho

880
1
15
26

Where? I cant see where you are redeclaring. – Marcelo Rodovalho Apr 18 '13 at 17:00
1

See [Dan's answer](http://stackoverflow.com/a/16088927/363701). This code: `jQuery('#letter_input_form').submit(function() {...` gets run every time `.box.unknown` is clicked, adding a new event handler to `#letter_input_form`. – Zach Lysobey Apr 18 '13 at 18:05
On yeah, really. Anyway, the .live deprecated. – Marcelo Rodovalho Apr 18 '13 at 18:20

Javascript scope -- var vs global

3 Answers3