What's wrong with defining JavaScript variables within if blocks?

Question

I have some code like this:

if (condition) {
var variable = blah;
}

if (differentcondition) {
var variable = blah;
}

Is this correct?

I assume that the variable wouldn't be assigned if the condition doesn't return true.

JSLint keeps on telling me, variable already defined.

Am I doing this wrong?

Thanks.

OK, Here's my actual usecase, i'm doing event delegation like this:

$("#container").click(function (event){ 

 if ($(event.target).is('img.class1')) {
  var imagesrc = $(event.target).attr('src');
  // Do something with imagesrc
 }

 if ($(event.target).is('img.class2')) {
  var imagesrc = $(event.target).attr('src');
  // Do something with imagesrc
 }

 // This condition is mutually exclusive to the above 2
 if ($(event.target).is('img.class3')) {
  var imagesrc = $(event.target).attr('src');
  // Do something with imagesrc
 }

 // This condition is mutually exclusive to 1 and 2 but not to 3
 if ($(event.target).is('img.class4')) {
  var imagesrc = $(event.target).attr('src');
  // Do something with imagesrc
 }

});

Actually these 2 classes are not mutually exclusive.

This works for me, but is it correct?

The answers were very informative, but I still don't understand how I should set up the variables here.

Actually I also want to say that certain conditions are mutually exclusive, and certain conditions are not.

How should I structure this?

I probably should've used this example from the beginning.

Answer 1

This is because in JavaScript, variables only have different scope within function blocks. Unlike other languages, if-blocks do not have a different scope in JavaScript.

In your case, JSLint would tell you the variable is already defined because it is possible that both conditions are true, in which case you'd be overwriting your variables.

Here is an example of how scopes work in JavaScript:

function Something() {
    var thing1 = "hello";

    (function() {
        var thing2 = "world";
    })();

    // This line will throw an exception because "thing2" is undefined in this scope
    // It only exists within the scope of the function executed above
    alert(thing1 + thing2);
}

function SomethingElse() {
    var thing1 = "hello";

    if(true) {
        var thing2 = "world";
    }

    // This line will work because thing2 is not limited to the if-block
    alert(thing1 + thing2);
}

In the first example, I'm not 100% sure if it will throw an exception, it may just see "thing2" as undefined. I haven't tested it, but hopefully you get the idea.

Answer 2

Are the two if-conditions mutually exclusive? Perhaps you want to use the "else" control structure:

if (condition) {
    var variable = blah;
}
else if (differentcondition) {
    var variable = blah;
}

This means that if the first condition is true, then the second part of the code will never be executed.

In general, it's better to declare variables within the structure they are being used in. If the variable should only exist within the if statement, declaring it there is fine, otherwise, move it outside like so:

var variable;
if (condition) {
    variable = blah;
}
else if (differentcondition) {
    variable = blah;
}
// continue to use variable here...

Note: if you're using separate ifs without the else, then you'll need to set variable to a default value first.

Answer 3

Only functions create inner scope. It gets even hairier like in this example.

var x = 2;
function foo() {
    alert(x); // will alert undefined?!?

    var x = 4;
}
foo();
alert(x); // will alert 2

The first alert will actually say undefined. Why? Because all variable initializations that occur in the function get initialized to undefined at the start of the function. Which is why Crockford says to initialize all variables as early as possible.

So your code should probably look like this:

$("#container").click(function (event){ 
        var imagesrc;
        if ($(event.target).is('img.class1')) {
                imagesrc = $(event.target).attr('src');
                // Do something with imagesrc
        }

        if ($(event.target).is('img.class2')) {
                imagesrc = $(event.target).attr('src');
                // Do something with imagesrc
        }

        // This condition is mutually exclusive to the above 2
        if ($(event.target).is('img.class3')) {
                imagesrc = $(event.target).attr('src');
                // Do something with imagesrc
        }

        // This condition is mutually exclusive to 1 and 2 but not to 3
        if ($(event.target).is('img.class4')) {
                imagesrc = $(event.target).attr('src');
                // Do something with imagesrc
        }

});

Answer 4

Because javascript has something called "Hoisting" which makes your code do things it doesn't look like it should be doing. Basically, that means a javascript interpreter will move all var declarations, regardless of where they are in the body of a function, to the top of the function body. Then it will move all function definitions to the top, just under all the vars. Then it will finish compiling the function.

Putting a var inside an if statement is not against "the rules" of the language, but it means that, because of var hoisting, that var will be defined regardless of whether the if statement's condition is satisfied.

Keep in mind also that hoisting does not include the assignment, so as others have pointed out, the var declarations will be moved to the top, and left undefined until they're assigned later.

This is an example of a feature that must have seemed like a good idea at the time, but it has turned out to be more confusing than helpful.