views:

87

answers:

4

Hi, I'm having trouble resolving a scope issue with my javascript. I have an array, dog[] that is defined from JSON, that I need access to from inside a nested function.

function blah(json) {
 for (var u = 0; u < json[0][1][u].length; u ++ ) {
    var dog = 'k' + json[0][1][u].doggies;
    console.log(dog); // prints array of doggie strings
    $('#puppy').click(function(dog) {    // dog is passed in the function
       console.log(dog); // Syntax error, unrecognized expression: #[object Object]
       $('#' + dog).css('display, 'none');
     });
   }
 }

when I dont pass dog into the click function: i get:

 $('#puppy').click(function() {
  console.log(dog) // (12)  main.js:122k4c812e3a7275e10331000000 - this is the last value in the array - from safari console
  $('#' dog).css('display', 'none);
   }

Does anyone have any suggestions to get the array with every element passed into the click function? Or am i calling the css method incorrectly to hide those divs?

A: 

You can't pass the dog value into the jquery click event as you have done there. The click function signature is:

$(object).click(function(){


});

You can't pass dog in like this. Even if the function expected a parameter, naming it dog would cause issues. You may need to store the values of dog in a more global scope so that when the click event occurs, you still have access to it.

KP
Ever heard of closures?
strager
Shadowing.... but in this case `dog` as an argument actually represents the `event`, since it's the first argument of a call back in `.click()`.
Peter Ajtai
@Peter: good point.
KP
@KP, I'm sorry; I misread your answer. (I missed two words and that totally threw off the meaning... My bad!)
strager
+1  A: 

Why not just give the doggies a class .dog and hide them when #puppy is clicked?

$("#puppy").click(function() {
    $(".dog").hide();
});

Or since your dog's IDs seem to start with k, you might consider something like this:

$("#puppy").click(function() {

    // hide everything with ID beginning with 'k'
    $('[id^=k]').hide();
});
karim79
Such a simple solution compared to everyone else. +1 for using a CSS class as selector.
KP
+2  A: 

Problem 1

Closures bind the entire function's scope, and not individual variables or values.

Take this code for example:

function foo() {
    var i, func;

    for (i = 0; i < 10; ++i) {
        if (i == 0) {
            func = function () {
                alert(i);
            }
        }
    }

    func();
}

foo();

You may expect foo to cause 0 to be alerted. However, the value of i has changed since the function assigned to func was created; the call to func alerts "10".

Here is another example illustrating the concept:

function foo() {
    var i = 42;

    function func() {
        alert(i);
    }

    for (i = 0; i < 10; ++i) {
        // do nothing
    }

    func();
}

foo();

Try to figure out what will be alerted, and run the code as a test.

Problem 2

The second problem is that variables are bound at the function scope (and not the block scope as you expect).

Take this code:

function foo() {
    var i;

    for (i = 0; i < 10; ++i) {
        var j = i;
    }

    alert(j);
}

foo();

You may expect this code to alert "undefined", throw a run-time error, or even throw a syntax error. However, "10" is alerted. Why? In JavaScript, the above code is translated into effectively:

function foo() {
    var i;

    var j;

    for (i = 0; i < 10; ++i) {
        j = i;
    }

    alert(j);
}

foo();

It should be more clear from this example that "10" is indeed alerted.

Solution

So how do you fix your problem? The simplest way is to change your logic: instead of attaching one event handler per dog, attack one event handler per collection of dogs. For example:

function blah(json) {
    $('#puppy').click(function () {
        var u, dog;

        for (u = 0; u < json[0][1][u].length; u++) {
            dog = 'k' + json[0][1][u].doggies;

            console.log(dog);

            $('#' + dog).css('display', 'none');
        }
    });
}

If you're interested in the "proper" transformation of your existing code (i.e. having the same behaviours, except with the bug fixed), I can give you an example of that as well. However, the solution I gave above is a much better solution and results in cleaner code.

strager
+1. Good answer. Rare to find people answering this well.
balupton
@strager - The first argument of a call back in `.click()` is the `event` that happened, so you should remove `dog` from `function(dog) {` in your solution.... just so as not to confuse the issue.
Peter Ajtai
THANK YOU SO MUCH! Very helpful explanation. I will study your example in more depth when I'm less sleep deprived.
fordays
@Peter Ajtai, Thanks for catching that. I copy-pasted the original code and basically just swapped two lines. I'll fix it.
strager
+2  A: 

Important Note:

You forgot to close your quote. This:

$('#' + dog).css('display, 'none');

Should be:

$('#' + dog).css('display', 'none');

An Improved Loop:

There are several problems with your script. I'll concentrate on the overall logical structure of the loop.

Instead of attaching many handlers to .click(), just attach one handler that iterates over you JSON using jQuery's .each(). The first argument of the callback of .each() is the index number and the second argument is the value. You can make use of those 2 by naming the arguments or by using arguments[0] and arguments[1]. I show the former method below:

I've added some more test output for demonstration purposes:

function blah(json) {
     $('#puppy').click(function() { 

          // iterate over each json[0][1]
        $.each(json[0][1], function(index, value) {

              // Your original 2 lines
            console.log(value);
            $('#' + value).css('display', 'none');

              // This is just test output, so you can see what is going
              // on.
            $("body").append("Number " + index + " is " + value ".<br/>");
        });
     });
}     
Peter Ajtai