"this" doesn't resolve as expected when I put my jquery dom manipulation methods in to a namepsace?

Question

I've managed to avoid using namespace in most of my javascript dev to date, but I'm beginning to see the light through reading helpful articles like this one

I'm following Maeagers technique found here to create my namespace as shown below -

var newAndImproved = {

//div to put loaded content into
targetDiv: "",

//loads the initial content based on an anchor tag having a class named 'chosen'
loadInitialPage: function() {
    var chosenLink = $("#subnavigation a.chosen");
    newAndImproved.loadPartial(chosenLink);
},

loadPartial: function(clickedLink) {
    var theUrlToLoad = $(clickedLink).attr("href");
    $(newAndImproved.targetDiv).load(theUrlToLoad, function(response, status, xhr) {
        if (status == "error") {
            var msg = "Sorry but there was an error: ";
            $(targetDiv).html(msg);
        }
    });
},

removeClassFromSubNav: function() {
    $("#subnavigation a").removeClass('chosen');
},

addChosenClassToNav: function(chosenLink) {
    $(chosenLink).addClass('chosen');
},

bindMethodsToNavigation: function() {
    $("#subnavigation a").bind('click', function(event) {
        var chosenLink = event.target;
        newAndImproved.removeClassFromSubNav();
        newAndImproved.loadPartial(chosenLink);
        newAndImproved.addChosenClassToNav(chosenLink);
        return false;
    });
}

};

I've called that namespace like this -

$(document).ready(function() {
newAndImproved.targetDiv = $("#subcontent");
newAndImproved.loadInitialPage();
newAndImproved.bindMethodsToNavigation();
});

I'm sure you've noticed that I'm referencing the namespace within itself rather than just using 'this'. I assume this is incorrect. But when I use 'this' the binding part will not work.

Strangely, the loadInitialPage method will work when using 'this'.

Any idea what I'm doing wrong?

Thanks in advance.

Answer 1

You need to use the that trick. this gets re-assigned inside new scopes, including the anonymous function you are binding.

Consider this code:

function say(x){ alert(x); }

var myscope = {

    label : "myscope (toplevel)",

    func1 : function() {
        say("enter func1");
        var label = "inner func1";
        say("  this.label: " + this.label);
        say("  label: " + label);
    },

    func2 : function() {
        var label = "inner func2";
        say("enter func2");
        this.func1();
    }
};

myscope.func2();

If you run this, it will behave nicely, and the references to this.xxx all succeed as you would like. However, if you add a func3 like this:

    func3 : function() {
        setTimeout( function(){
            var label = "anonymous func";
            say("enter anon func");
            this.func1();
        }, 2);
    }

...it will not work as you might imagine. Because the anonymous function is defined in no explicitly-specified scope, the this within it refers to the top level object, window. And there is no func1 as a child of window (no top level object named func1).

For the same reason, the this within your anon function that you use in the call to bind() will fail. In that case you can refer to the "namespace" object directly, or you can use a closure (some people call it "the that trick"):

    bindMethodsToNavigation: function() {
        var that = this;
        $("#subnavigation a").bind('click', function(event) {
            var chosenLink = event.target;
            that.removeClassFromSubNav();
            that.loadPartial(chosenLink);
            that.addChosenClassToNav(chosenLink);
            return false;
        });
    }

The this within the scope of bindMethodsToNavigation refers to newAndImproved. The this within the anonymous function will refer to window. Using a closure allows you to refer to the thing you want. (editorial comment: I personally find the name that to be cute and entirely unhelpful. I like to use abbreviated names, in your case maybe something like nai to refer to newAndImproved)

Incidentally, you could also do this:

    clickHandler : function(event) {
        var chosenLink = event.target;
        this.removeClassFromSubNav();
        this.loadPartial(chosenLink);
        this.addChosenClassToNav(chosenLink);
        return false;
    },

    bindMethodsToNavigation: function() {
        $("#subnavigation a").bind('click', clickHandler);
    },

...and if they are both members of the newAndImproved object, then the this within clickHandler will be resolved to the newAndImproved object. The key here is that clickHandler is not an anonymous, top-level function; it belongs to newAndImproved and when this is used within it, it resolves appropriately. Even more, you can just drop the use of this within clickHandler; it is not incorrect, but it is unnecessary. Some people like to add it for emphasis to those who might read the code later.

---

EDIT

One more note on this - on the decision whether to use anonymous functions or named functions as event handlers... Anon functions are so convenient and easy, and everybody uses them. What I find is that in Firebug or the IE Javascript Debugger, when I look at a stack trace I get a loooong list of anon functions, and it's difficult to figure out just what the chain of functions is. Especially with AJAX and async handlers being fired in response to UI events, and so on. To provide some additional visibility at debug time, I sometimes use named functions (put into namespaces), and the callstack will show a few named fns along interspersed with a few anon fns. This sometimes helps. Just a small thing, though.

Answer 2

I don't see the need for namespaces at all. This looks like a very specific implementation (and not a reusable class). Wouldn't this be much simpler:

$(document).ready(function() {
    var targetDiv = $("#subcontent"),
    loadInitialPage = function() {
        ...
    },
    loadPartial = function(clickedLink) {
        ...
    },
    removeClassFromSubNav: function() {
        ...
    },
    addChosenClassToNav = function(chosenLink) {
        ...
    },
    bindMethodsToNavigation = function() {
        ...
    };

    loadInitialPage();
    bindMethodsToNavigation();
});

Simpler because you can reference all functions without namespace now. No need for this. or namespacename..

Why separate the namespace/function declarations from the document.ready stuff?

edit
You should read this. It's more than a few lines, but it's very readable and very useful.

edit
It's alright to put a lot of stuff in document.ready, because that's only executed once.