Is that a bad Javascript practice I'm doing here?

Question

for some reason I do that every time because I find it clean. I declare variables on top to use them below. I do that even if I use them only once.

Here is an example (using jQuery framework) :

$("#tbListing").delegate("a.btnEdit", "click", function(e) {
    var storeId = $(this).closest("tr").attr("id").replace("store-", ""),
        storeName = $(this).closest("tr").find("td:eq(1)").html(),
        $currentRow = $(this).closest("tr");

    $currentRow.addClass("highlight");

    $("#dialogStore")
        .data("mode", "edit")
        .data("storeId", storeId)
        .data("storeName", storeName)
        .dialog( "open" );

    e.preventDefault();
});

I tend to do that in PHP too. Am I right if I believe it's not very memory efficient to do that ?

Edit: Thank you for all the answers. You have all given good answers. About that code optimisation now. Is that better now ?

            $("#tbListing").delegate("a.btnEdit", "click", function(e) {
                var $currentRow = $(this).closest("tr"),
                    storeId = this.rel, /*storing the storeId in the edit button's rel attribute now*/
                    storeName = $currentRow.find("td:eq(1)").html();

                $currentRow.addClass("highlight");

                $("#dialogStore")
                    .data("info", {
                        "mode" : "edit",
                        "storeId" : storeId,
                        "storeName" : storeName
                    }) /*can anyone confirm that overusing the data isn't very efficient*/
                    .dialog( "open" );


                e.preventDefault();
            });

Answer 1

Sorry, are you asking if it's OK to declare variables even if you're using them once?

Absolutely! It makes the code a million times more readable if you name things properly with a variable. Readability should be your primary concern. Memory efficiency should only be a concern if it proves problematic.

As Knuth said,

We should forget about small efficiencies, say about 97% of the time: premature optimization is the root of all evil.

If you're asking more about declaring the variables at the beginning of the function, rather than where they are first used, then Emmett has it right - Crockford recommends doing this in JavaScript to avoid scope-related confusion. Whether it's worth it in PHP is a purely subjective question I'd say, but there's nothing wrong with keeping your PHP and JS coding styles similar.

---

One more CS quote (from Abelson and Sussman's SICP):

programs must be written for people to read, and only incidentally for machines to execute.

Answer 2

It's not bad practice.

The var statements should be the first statements in the function body.

---

JavaScript does not have block scope, so defining variables in blocks can confuse programmers who are experienced with other C family languages. Define all variables at the top of the function.

http://javascript.crockford.com/code.html

Answer 3

Declaring variables at the top is a good thing to do. It makes the code more readable. In your particular example, you could replace $(this).closest('tr') witha variable, as suggested int eh comments, but in general I find code with descriptive variable names all in one place very readable.

Answer 4

nah, I'd say you're doing exactly the right thing.

As @Caspar says, you could simplify your code by setting $currentRow first and using that instead of $(this).closest("tr") in the other two lines. And there may be a few other things you could improve. But setting vars at the begining of a function the way you've done it is absolutely a good thing.

Particuarly good because you've done it inside the function, so they're local variables, which means they get thrown away at the end of the function, so no memory usage issues there.

If you'd set them as global vars, it might have been more of an issue, although to be honest even then, since you're just setting pointers to an existing object, it wouldn't be using a huge amount of memory even then (though it would be polluting the global namespace, which isn't good)