Question about best practice with JS objects and internal method calls

example.js

messageOfTheDay = function (appName) {
	this.appName = appName;
	if (this.lastMessageText === undefined) {
		this.lastMessageText = "";
		}
	this.sayHello = function () {
		console.log ("hello");
		}
	this.checkForUpdate = function () {
		var urlTextFile = "http://1999.io/testing/motd/" + this.appName + ".txt";
		var motd = this;
		readHttpFile (urlTextFile, function (s) {
			if (s !== undefined) {
				if (s !== this.lastMessageText) {
					this.lastMessageText = s;
					console.log ("messageOfTheDay.checkForUpdate: s == " + s);
					motd.sayHello ();
					}
				}
			});
		}
	};

Dear braintrustees...

I haven't been using object-oriented JavaScript objects very much, so I thought I'd use them for this little project I have that will display a "message of the day" in one of my browser-based JS apps.

I have a question...

On line 17, consider the call to motd.sayHello.

That call works because at a higher level I copy the value of this into the variable motd.

It took me a while to get there, I was calling this.sayHello and it would come back saying the function sayHello wasn't defined.

Here's this question..

It this the best way to do this?

Thanks!

Dave

There's a cleaner way to do this. You "require" the template object, then you call "new" on the "constructor" to get a new version of the object. This way you can instantiate multiple copies of the object and each will have their own private instance variables. The way you currently have it, there will only be "global" instance vars. Here's some sample code:

   //Obj.js
    function Obj (args) {
        if ( !(this instanceof Obj) ) {
            return new Obj (args);
        }
        // set instance variables here
        this.instanceVar = args;
    }

    Obj.prototype.someMethod = function someMethod () {
        //some someMethod logic here, e.g.:
        return this.instanceVar;
    }

     module.exports = Obj;

Then you can instantiate this object like:

const OBJ = require ('Obj');
var myObj = new OBJ ('instance specific data');
var myOtherObj = new OBJ ('some different data');

console.log (myObj.someMethod ()); // 'instance specific data'

Up until the introduction of ES6, that's pretty much how you have to do it. ES6 introduces "arrow functions" which do not rebind this so it still refers to the this in the surrounding scope.

However, I'm confused by the use of this in the callback function defined an the second argument on line 12. What object is bound tothis when it is invoked? Is https://github.com/scripting/river4/blob/master/lib/utils.js#L392 the definition of ReadHttpFile? If so, it doesn't appear to explicitly pass a this value when calling the call back.

I'll assume that you want those this reference to all reference motd. In that case lines 12-21 can be replaced with:

        readHttpFile (urlTextFile,  s => {
            if (s !== undefined) {
                if (s !== this.lastMessageText) {   //this is the same as checkForUpdate's this
                    this.lastMessageText = s;
                    console.log ("messageOfTheDay.checkForUpdate: s == " + s);
                    this.sayHello ();     //motd not needed
                    }
                }
            });
}

Of course, if ES6 arrow functions are available, ES6 class declarations are probably also available. So you could write the whole thing something like:

class messageOfTheDay  {
  constructor(appName) {
    this.appName = appName;
    if (this.lastMessageText === undefined) {
        this.lastMessageText = "";
    }
  }

   sayHello() {
      console.log ("hello");
   }

   checkForUpdate() {
      var urlTextFile = "http://1999.io/testing/motd/" + this.appName + ".txt";
      readHttpFile (urlTextFile, s  => {
         if (s !== undefined) {
            if (s !== this.lastMessageText) {
               this.lastMessageText = s;
               console.log ("messageOfTheDay.checkForUpdate: s == " + s);
               self.sayHello ();
            }
         }
      });
   }
}

Allen, you're absolutely right about this.lastMessageText.

It clearly refers to an object that belongs to the callback to readHttpFile.

It's a mistake -- and I'll need to think about how it works and fix it.

BTW, re ES6, I'm not using any ES6 features yet.

I found a subset of JS that works for me, and then proceeded to develop the functionality I wanted. There's a lot of that so it hasn't left much time for exploring new language functionality. That's the way my brain works. It's why I don't like people figuring out how to fix "callback hell" -- I've made my peace with it, and would prefer not to learn another way to do something I already know how to do.

At age 61, I'm kind of "set in my ways" in this regard. ;-)