[trickstar34]

OK so I created an updating clock script that works (the first set of code) but it only works for one instance so I decided to change it where it will support multiple instance so I wrote the code with embedded functions (second set of code). The first code works and the second code is pretty much just the same code but written using [icode]this.example = function() {[/icode]. Can someone help me fix it? This is my first attempt designing a script using the "this" feature and I'd like to know whats wrong. I have it set up where you can choose to make the code display the servers time (by embedding server-side values for the time), using the clients (by using the Javascript date() function) or by setting your own values into the parameters of the main function.

Original (code that works):

			// v1.0
                        var clockElement = document.getElementById("Clock");
                        var clockZone = "(CST)";
                        var clockHours = 2;
                        var clockMinutes = 8;
                        var clockSeconds = 21;
                        var clockMilleseconds = 1000 - 321;
                        function clockDisplay() {
                            if (clockSeconds > 59) {
                                clockSeconds = 0;
                                if (++clockMinutes > 59) {
                                    clockMinutes = 0;
                                    clockHours = (clockHours + 1) % 24;
                                    }
                                }
                            var hours = clockHours;
                            var ampm = (hours >= 12) ? " PM " : " AM ";
                            if (hours > 12) { hours -= 12; }
                            else if (hours == 0) { hours = 12; }
                            clockString = (hours < 10 ? "0" : "") + hours + ":" + (clockMinutes < 10 ? "0" : "") + clockMinutes + /*":" + (clockSeconds < 10 ? "0" : "") + clockSeconds + */ampm + clockZone;
                            clockElement.innerHTML = clockString;
                            ++clockSeconds;
                        }
                        function clockUpdate() {
                            clockDisplay();
                            setInterval(clockDisplay, 1000);
                        }
                        clockDisplay();
                        setTimeout(clockUpdate, clockMilleseconds);

New (code that doesn't work):

                        function Clock(elementID, TimeZone, hour, minutes, second, millesecond) {
                            this.clockElement = document.getElementById(elementID);
                            this.clockZone = "(" + TimeZone + ")";
                            this.clockHours = hour;
                            this.clockSeconds = second;
                            this.clockMilleseconds = 1000 - millesecond;
                            this.firstIntervalReached = false;
                            this.update = false;
                            this.display = function() {
                                if (this.clockSeconds > 59) {
                                    this.clockSeconds = 0;
                                    if (++this.clockMinutes > 59) {
                                        this.clockMinutes = 0;
                                        this.clockHours = (clockHours + 1) % 24;
                                        }
                                }
                                var hours = this.clockHours;
                                var ampm = (hours >= 12) ? " PM " : " AM ";
                                if (hours > 12) { hours -= 12; }
                                else if (hours == 0) { hours = 12; }
                                clockString = (hours < 10 ? "0" : "") + hours + ":" + (clockMinutes < 10 ? "0" : "") + clockMinutes + ":" + (clockSeconds < 10 ? "0" : "") + clockSeconds + ampm + clockZone;
                                clockElement.innerHTML = clockString;
                                if (this.update == true) {
                                    ++this.clockSeconds;
                                    if (this.firstIntervalReached == false) {
                                        setTimeout(this.display, this.clockMilleseconds);
                                    }
                                    else {
                                        setTimeout(this.display, 1000);
                                    }
                                }                  
                        }
                        var clock = new Clock("Clock", "CST",  2, 5, 23, 321);
                        clock.update = true;
                        clock.display();

[JackOfAllTrades]

I'm going to be perfectly honest with you: I am not a Javascript guru, and really don't have much grasp on scoping rules in the language. I try to avoid JS as much as possible; hell, I try to avoid front-end work as much as possible. But I messed around with this, and this is what I came up with, which I *think* works (at least in Firefox), but I'm not positive it's exactly right.

Hopefully someone more JS-savvy will come along and give a better explanation (or proper code).

<!doctype html>
<html>
<head>
<script type="text/javascript">
function Clock(elementID, TimeZone, hour, minutes, second, millesecond) {
    var theObj = this; // I believe this (pardon the pun) is needed because "this" takes on a different context within the display method, so we save a reference to "this" in "theObj" for reference.
    this.clockElement = document.getElementById(elementID);
    this.clockZone = "(" + TimeZone + ")";
    this.clockHours = hour;
    this.clockSeconds = second;
    this.clockMinutes = minutes;
    this.clockMilleseconds = 1000 - millesecond;
    this.firstIntervalReached = false;
    this.update = false;
    this.display = function() {
        if (theObj.clockSeconds > 59) {
            theObj.clockSeconds = 0;
            if (++theObj.clockMinutes > 59) {
                theObj.clockMinutes = 0;
                theObj.clockHours = (clockHours + 1) % 24;
            }
        }
        var hours = theObj.clockHours;
        var ampm = (hours >= 12) ? " PM " : " AM ";
        if (hours > 12) { hours -= 12; }
        else if (hours == 0) { hours = 12; }
        clockString = (hours < 10 ? "0" : "") + hours + ":" + (theObj.clockMinutes < 10 ? "0" : "") + theObj.clockMinutes + ":" + (theObj.clockSeconds < 10 ? "0" : "") + theObj.clockSeconds + ampm + theObj.clockZone;
        theObj.clockElement.innerHTML = clockString;
        if (theObj.update == true) {
            ++theObj.clockSeconds;
            if (theObj.firstIntervalReached == false) {
                setTimeout(theObj.display, theObj.clockMilleseconds);
            }
            else {
                setTimeout(theObj.display, 1000);
            }
        }
    }
};
function initClock() {
	var clock = new Clock("Clock", "CST",  2, 5, 23, 321);
	clock.update = true;
	clock.display();

	var clock2 = new Clock("Clock2", "EST", 3, 5, 23, 321);
	clock2.update = true;
	clock2.display();
}
// After DOM loaded
if (document.addEventListener)
  document.addEventListener("DOMContentLoaded", initClock, false);
</script>
</head>
<body>
<div id="Clock"></div>
<div id="Clock2"></div>
</body>
</html>

[trickstar34]

This is my code now and it still doesn't work Theres something else wrong but I think you did help though.

(JS file)

function Clock(elementID, TimeZone, hour, minute, second, millesecond) {
    var objectVar = this;
    this.clockElement = document.getElementById(elementID);
    this.clockZone = "(" + TimeZone + ")";
    this.clockHours = hour;
    this.clockSeconds = second;
    this.clockMinute = minute;
    this.clockMilleseconds = 1000 - millesecond;
    this.firstIntervalReached = false;
    this.update = false;
    this.display = function() {
      if (objectVar.clockSeconds > 59) {
         objectVar.clockSeconds = 0;
         if (++objectVar.clockMinutes > 59) {
         objectVar.clockMinutes = 0;
         objectVar.clockHours = (clockHours + 1) % 24;
         }
      }
      var clockString;
      var hours = objectVar.clockHours;
      var ampm = (hours >= 12) ? " PM " : " AM ";
      if (hours > 12) { hours -= 12; }
      else if (hours == 0) { hours = 12; }
      clockString = (hours < 10 ? "0" : "") + hours + ":" + (objectVar.clockMinutes < 10 ? "0" : "") + objectVar.clockMinutes + /*":" + (objectVar.clockSeconds < 10 ? "0" : "") + objectVar.clockSeconds + */ampm + objectVar.clockZone;
      objectVar.clockElement.innerHTML = clockString;
      if (objectVar.update == true) {
         ++objectVar.clockSeconds;
         if (objectVar.firstIntervalReached == false) {
            objectVar.firstIntervalReached = true;
            setTimeout(objectVar.display, objectVar.clockMilleseconds);
         }
         else {
         setTimeout(objectVar.display, 1000);
         }
      }                  
};

(Head script):

                function initializeClock() {
                var clock = new Clock("Clock", "CST", 1, 5, 45, 321);
                clock.update = true;
                clock.display();
                }
                if (document.addEventListener) {
                    document.addEventListener("DOMContentLoaded", initializeClock, false);
                }

(Body HTML):

<div id="Clock" align="right"><span style="color: red; font-size: small;">Error running Javascript</span></div>

This post has been edited by trickstar34: 30 June 2012 - 03:51 PM

[JackOfAllTrades]

Quote

This is my code now and it still doesn't work

You need to elaborate on what you mean by "it still doesn't work". In what way? What does it do that you don't expect, or what does it not do that you do expect? Is it giving you errors? Do you use Firebug or some other Javascript debugger? If not, you should use one.

[trickstar34]

JackOfAllTrades, on 01 July 2012 - 04:22 AM, said:

Quote

This is my code now and it still doesn't work

You need to elaborate on what you mean by "it still doesn't work". In what way? What does it do that you don't expect, or what does it not do that you do expect? Is it giving you errors? Do you use Firebug or some other Javascript debugger? If not, you should use one.

It doesn't place the time into the element, it says error with Javascript which would get removed if the code was working correctly.

[Atli]

What does the Error Console tell you? If there is a Javascript error, it will be detailed there.

I see three errors in the last code you posted. Well, more like two errors and a couple of things that need improving.

First, your indentation is a little messed up. Each code block should be indented one tab (or, typically, four spaces) deeper than the block it belongs to. Of course there are different indent styles, but this is a popular one. - It makes the code easier to read and makes the kind of errors you are facing much easier to spot.

Which brings us to your first error: You are missing the end } for the Clock.display function. That should cause a syntax error that would be displayed in the aforementioned Error Console.

The second error is a variable name mismatch. On line #7 you define a this.clockMinute attribute, but inside your Clock.display function you always reference it as objectVar.clockMinutes. (Notice the added "s" to the end.) - This would not generate an error in the console, but would show "undefined" or "NaN" in the generated output, in place of the minutes.


One final note. Line #24 is a real monster. It would be better for readability to break it down into like three lines, one for hours, one for minutes and then one for the AM/PM and timezone info. - Better readability means easier debugging and maintenance in the future, which is of far greater value than reducing the line count by two is.

[JackOfAllTrades]

Atli, is this

var theObj = this; // I believe this (pardon the pun) is needed because "this" takes on a different context within the display method, so we save a reference to "this" in "theObj" for reference.

the correct explanation?

[Atli]

Partially. The meaning of this doesn't really change for the display method until it is called out of context. This usually happens when functions are used as callbacks.

The problem with how this is handled in Javascript is the fact that object methods are in fact not methods at all, but rather individual functions attached to an object attribute. Meaning that they can be called by themselves, without them being aware of the object they are attached to.

When you call a function on an object like so: obj.func(), then the this variable inside the function will be the object. However, if you call the function by itself: obj.func.call() (or other variations on that), then the this variable refers to the function itself. (Functions being objects, like everything else.)

So when you do, for example:

function foo() {
	this.value = "Hello, Scope Hell!";
	this.display = function() {
		var op = document.getElementById("output");
		op.innerHTML = this.value;
		setTimeout(this.display, 1000);
	}
}

var obj = foo();
obj.display();

The initial obj.display() call will update the "output" element with the proper value, but the following call to it by the setInterval function will make the function itself the this value, resulting in undefined when you try to access the obj attributes.

The typical solution to this is what you suggested:

function foo() {
	var thisObj = this;
	this.value = "Hello, Scope Hell!";
	this.display = function() {
		var op = document.getElementById("output");
		op.innerHTML = thisObj.value;
		setTimeout(thisObj.display, 1000);
	}
}

Because of the way scopes work in Javascript, the thisObj variable will be available to the display function no matter where it is called from or what it's this value is set to.

[Dormilich]

if a more modern version of Javascript is supported (ECMAScript 5th edition, I think) the function’s bind() method can be used as well.

function foo() {
	this.value = "Hello, Scope Hell!";
	this.display = function() {
		var op = document.getElementById("output");
		op.innerHTML = this.value;
		setTimeout(this.display.bind(this), 1000);
	}
}

respectively if foo() is used more than once as constructor:

function Foo() {
	this.value = "Hello, Scope Hell!";
}
Foo.prototype.display = function() {
	var op = document.getElementById("output");
	op.innerHTML = this.value;
	setTimeout(this.display.bind(this), 1000);
}

[trickstar34]

Atli, on 01 July 2012 - 03:28 PM, said:

What does the Error Console tell you? If there is a Javascript error, it will be detailed there.

I see three errors in the last code you posted. Well, more like two errors and a couple of things that need improving.

First, your indentation is a little messed up. Each code block should be indented one tab (or, typically, four spaces) deeper than the block it belongs to. Of course there are different indent styles, but this is a popular one. - It makes the code easier to read and makes the kind of errors you are facing much easier to spot.

Which brings us to your first error: You are missing the end } for the Clock.display function. That should cause a syntax error that would be displayed in the aforementioned Error Console.

The second error is a variable name mismatch. On line #7 you define a this.clockMinute attribute, but inside your Clock.display function you always reference it as objectVar.clockMinutes. (Notice the added "s" to the end.) - This would not generate an error in the console, but would show "undefined" or "NaN" in the generated output, in place of the minutes.


One final note. Line #24 is a real monster. It would be better for readability to break it down into like three lines, one for hours, one for minutes and then one for the AM/PM and timezone info. - Better readability means easier debugging and maintenance in the future, which is of far greater value than reducing the line count by two is.

Thank you, it works now. I was completely unaware of the Error Consoles existence. And the code indentation is just what makes it easier for me to read I guess, the main thing was the this.display ending "}", but it works now so thanks for your help!