views:

94

answers:

2

I'm having difficulty with Javascript instance variables. I'm trying to make an easy way to refresh the chat page, looking for new messages. The AJAX call supplies a mid, or the lowest message id to be returned. This allows the call to only ask for the most recent messages, instead of all of them.

MessageRefresher.prototype._latest_mid;

function MessageRefresher(latest_mid) {
    this._latest_mid = latest_mid; // it thinks `this` refers to the MessageRefresher object
}

MessageRefresher.prototype.refresh = function () {
    refreshMessages(this._latest_mid); // it thinks `this` refers to the window
    this._latest_mid++;
}

function refreshMessages(latest_mid) {
    $.getJSON('API/read_messages', { room_id: $.getUrlVar('key'), mid: latest_mid }, function (messages) {
        for (var i = 0; i < messages[0].length; i++) {
            var newChild = sprintf("<li>%s: %s</li>", messages[1][i], messages[0][i]);
            $("#messages").append(newChild);
        }
    });

    var messageRefresher = new MessageRefresher(0);
    setInterval(messageRefresher.refresh, 1000);

This results in all the messages being printed out, over and over again.

I know it has other bugs, but the main issue I'm trying to work out right now is the use of the instance variable. Or is there another way I should be going about doing this?

UPDATE: Updated code to reflect answers, but it still doesn't work:

$(document).ready(function () {

    var messageRefresher = new MessageRefresher(0);
    setInterval($.proxy(messageRefresher, "refresh"), 1000);

});

function MessageRefresher(latest_mid) {
    this._latest_mid = latest_mid; // it thinks `this` refers to the MessageRefresher object
}

MessageRefresher.prototype.refresh = function () {
    refreshMessages(this._latest_mid); // it thinks `this` refers to the window
}

function refreshMessages(latest_mid) {
    var refresher = this;
    $.getJSON('API/read_messages', { room_id: $.getUrlVar('key'), mid: latest_mid }, function (messages) {
        if (messages != null) {
            $("#messages-loading-msg").hide();
            for (var i = 0; i < messages[0].length; i++) {
                var newChild = sprintf("<li>%s: %s</li>", messages[1][i], messages[0][i]);
                $("#messages").append(newChild);
            }
        }
        else {
            $("#messages-loading-msg").text("No messages here. Say anything...");
        }

        refresher._latest_mid = messages[2];
    });
    }

What am I still doing wrong? Does this really refer to the messageRefresher, or to the proxy?

UPDATE 2: Getting closer, but still not quite there:

$(document).ready(function () {

    $("#no-js-warning").empty();

    messageRefresher = new MessageRefresher(0);
    setInterval($.proxy(messageRefresher, "refresh"), 1000);

});

function MessageRefresher(latest_mid) {
    this._latest_mid = latest_mid; 
}

MessageRefresher.prototype.refresh = function () {
    var refresher = this;
    $.getJSON('API/read_messages', { room_id: $.getUrlVar('key'), mid: refresher._latest_mid }, function (messages) {
        if (messages != null) {
            $("#messages-loading-msg").hide();
            for (var i = 0; i < messages[0].length; i++) {
                var newChild = sprintf("<li>%s: %s</li>", messages[1][i], messages[0][i]);
                $("#messages").append(newChild);
            }
        }
        else {
            $("#messages-loading-msg").text("No messages here. Say anything...");
        }

        refresher._latest_mid = messages[2]; // break here
    });
}

The above code makes two AJAX calls, then stops. Also, when I break on the "break here" line labeled above, Firebug does not show values for refresher or _latest_mid. Am I passing refresher into the closure correctly?

UPDATE 3: Now it no longer stops making AJAX calls. Perhaps the reason that it sometimes makes multiple calls is that setInterval() fires off multiple requests before the first request has returned and has updated _latest_mid.

How can I use the JQuery Ajax framework to prevent this from happening?

+2  A: 

Try this:

setInterval($.proxy(messageRefresher, "refresh"), 1000);

The problem is that you need to make sure that your object (messageRefresher) is used as the "context" for the call to the "refresh" function. The mere fact that it's declared as being part of the prototype does not ensure that "this" will always be an instance of the object. In fact, "this" could be pretty much anything.

The (fairly new) "proxy" method in jQuery returns a reference to a helper function that'll call your function ("refresh") with the object you need as the context (that is, "this" inside "refresh").

Also that first line, where you just sort-of mention that instance variable, you don't need that; it doesn't really do anything.

edit More explanation.

The "this" variable inside of any Javascript function, no matter how or where it's declared, gets its value upon each distinct invocation. It is not a permanent part of the function; in effect, it's like a somewhat special parameter passed to the function.

In your case above, you've got a line of code inside the function you pass to $.getJSON that refers to "this". What you want is for that "this" reference to mean your "messageRefresher" object, right? Well, as it stands, there's no reason for it to have that value.

I don't know why you've got "refreshMessages" split out like that. Make that be the "refresh" method. Then, save the "this" pointer at the outset, and you'll be able to refer back from the $.getJSON handler:

MessageRefresher.prototype.refresh = function () {
  var refresher = this;
  $.getJSON('API/read_messages', { room_id: $.getUrlVar('key'), mid: latest_mid }, function (messages) {
    if (messages.length > 0) {
        $("#messages-loading-msg").empty();
        for (var i = 0; i < messages[0].length; i++) {
            var newChild = sprintf("<li>%s: %s</li>", messages[1][i], messages[0][i]);
            $("#messages").append(newChild);
        }
        refresher._latest_mid = messages[2];
    }
    else {
        $("#messages-loading-msg").text("No messages here. Say anything...");
    }

  });
 }  
Pointy
Thanks, but it's still not totally working. See above?
Rosarch
Thanks again, but my code is still having issues.
Rosarch
+1  A: 

It's because you're taking the function and assigning it to an interval. You've passed the function object to the setInterval, so it doesn't keep track of its owner.

The function called from an interval will be in scope of the global object. This is how you fix that.

setInterval(function() {
    //this == window normally

    messageRefresher.refresh();
    //method invocation, so refresh will be bound
    //to the messageRefresher instance
}, 1000);
seanmonstar
That doesn't seem to do it. Firebug complains that your example is a "useless setInterval call", the return type of `messageRefresher.refresh()` != a function. Putting it in quotes gives the error "messageRefresher is not defined".
Rosarch
what? I know this works. and i just double checked, worked perfectly. You dont need any quotes. Are you leaving out the anonymous function part? dont do this: `setInterval(messageRefresher.refresh(), 1000);` that tries to set an interval using a function that is returned from refresh, which obviously does not.
seanmonstar