CODEWITHSUNDEEP
jqueryvariablesscoping
Julius
Julius

Reputation: 2864

Why is my jQuery variable empty?

I am new to jQuery and I ran into a problem that is probably easy to spot for most of the frontend gurus out here.

I created the following script

$(document).ready(function() {
    $("#facet_properties input[type='checkbox']").each(function() {
        $(this).click(function() {
            var queryString = getQueryString();
            $('body').load(window.location.pathname.concat(queryString));
        })
    });
});

function getQueryString() {
    var queryString = "";
    var queryStringBegun = false;
    $("#facet_properties input[type='checkbox']").each(function() {
        if ($(this).is(':checked')) {
            var checkboxFullName = $(this).attr('name');
            var checkboxName = checkboxFullName.substr(checkboxFullName.lastIndexOf('_') + 1);
            var concatenationCharacter = '&';
            if (!queryStringBegun) {
                concatenationCharacter = '?';
            }
            queryString.concat(concatenationCharacter).concat(checkboxName);
            queryStringBegun = true;
        }
    });

    return queryString;
}

I am trying to create a query string variable in the getQueryString method, but somehow queryString stays empty. When a checkbox is checked I see the statement queryString.concat(concatenationCharacter).concat(checkboxName); is reached and I see that 'concetenationCharacter' and 'checkboxName' have the correct value. So why does my function return ""?

This is probably a variable scoping issue or something. I am guessing it's something simple. Thanks!

Upvotes: 0

Views: 219

Answers (3)

Saurabh Bayani
Saurabh Bayani

Reputation: 3500

concat() returns a value....

collect that value ... line queryString.concat(concatenationCharacter).concat(checkboxName);

should be something like this

queryString = queryString.concat(concatenationCharacter).concat(checkboxName);

Upvotes: 1

giuliot
giuliot

Reputation: 163

I haven't tested it, but I think I would do something like

$(document).ready(function() {
    $("#facet_properties input[type='checkbox']").each(function() {
        $(this).click(function() {
            var checkboxFullName = $(this).attr('name');
            var checkboxName = checkboxFullName.substr(checkboxFullName.lastIndexOf('_') + 1);
            var concatenationCharacter = '&';
            if (!queryStringBegun) {
                concatenationCharacter = '?';
            }
            var queryString.concat(concatenationCharacter).concat(checkboxName);
            $('body').load(window.location.pathname.concat(queryString));
        })
    });
});

As I said, I have not tested it

Upvotes: -1

JJJ
JJJ

Reputation: 33163

concat() doesn't alter the original string. You have to assign the return value back to the variable: queryString = queryString.concat(concatenationCharacter).concat(checkboxName);

Using concat() is a bit of an overkill, though: you can concatenate strings simply with the + operator:

queryString += concatenationCharacter + checkboxName;

Not related to the problem, but you can shorten the code you have by moving the .is(':checked') check from the function to the selector:

$("#facet_properties input[type='checkbox']:checked").each(function() {
    ...

You also don't need to attach the click event to each element individually. You can again just use the selector. Using a variable in the event is also a bit unnecessary (unless you have more use for it in the actual app).

$(document).ready(function() {
    $("#facet_properties input[type='checkbox']").click(function() {
        $('body').load(window.location.pathname + getQueryString() );
    });
});

Upvotes: 1

Related Questions