Object-oriented timers

I'm just learning object-oriented Javascript, and I decided to try making some timers with it.

My object is below, heavily commented for your benefit.

function timer() {
this.startMins;
this.startSecs; //The time to start the timer at
this.mins;
this.secs; //The current amount of time displayed
this.action; //1 for count-up timers, -1 for count-down
this.msg; //Message to display when count-down reaches 0
this.prefix; //Prefix for IDs of timers
this.stopBtn = document.getElementById(this.prefix + "Stop");
this.startBtn = document.getElementById(this.prefix + "Start");
this.resetBtn = document.getElementById(this.prefix + "Reset");
this.display = document.getElementById(this.prefix + "Display");
this.intervalId; //Identifier for the interval to update timer
this.start = function() {
this.startBtn.disabled=1;
this.stopBtn.disabled=0;
this.resetBtn.disabled=0; //Disable necessary buttons
this.intervalId = setInterval("this.count()",1000); //Start timer
}
this.count = function() {
eval(this.secs += this.action); //Count up or down
if (this.secs >= 60) {
this.secs = 0;
this.mins++; //Convert to minutes/seconds
}
if (this.secs < 0) {
if (this.mins > 0) {
this.secs = 59;
this.mins--;
}
else {
alert(this.msg); //Alert msg if count-down is at 0
}
}
this.displayOutput();
}
this.stop = function() {
clearInterval(intervalId); //Stop timer
this.startBtn.disabled=0;
this.stopBtn.disabled=1; //Disable buttons
}
this.reset = function() }
this.startBtn.disabled=0;
this.stopBtn.disabled=1;
this.resetBtn.disabled=1;
this.mins = this.startMins;
this.secs = this.startSecs; //Reset timer to initial values
this.displayOutput(); //Update display to reflect changes
}
this.displayOutput = function() {
this.display.innerHTML = this.mins+":"+this.secs; //Display time
}
}

I'm unsure what I need to do next. Please read over my code, ensure that it is well-written before I develop any bad habits. Also, performance is the most important aspect of this object, so I would like to optimise it for performance as much as possible.

I've basically done all I can do on that object from the knowledge I have, and I believe that is the main code that I need. I'm not sure how I would go about making an actual object to use on my page.

The idea was that I could have HTML forms on my page that showed the timers.

<span id="xxDisplay"></span>
<input type="text" id="xxStop" value="Stop" />
<input type="text" id="xxStart" value="Start" />
<input type="text" id="xxReset" value="Reset" />

Notice that the prefix in this case would be "xx". Then I would initialise the object, and set all necessary values.

My main comment is that you have your functions defined in the object itself instead of in its prototype. You have

function constructor() {
this.func = function () {
/* blah */
};
}

This will create a copy of this anonymous function for every instance of the object even though (typically) that function will be the same regardless of the object. The way it is usually done is like this.

function constructor() {
}

constructor.prototype.func = function () {
/* blah */
};

Which will have just one version of the function for all the objects instead of one for each object.

david_kw

Ok, thanks. Do I call the function the same way (ie. objectName.functionName();)?

Yes it is called exactly the same way.

david_kw

timer.prototype.start = function() {
this.startBtn.disabled=1;
this.stopBtn.disabled=0;
this.resetBtn.disabled=0;
this.intervalId = setInterval('this.count()',1000);
}

I've run into a problem setting the interval. The interval causes the error "this.count is not a function". I've also tried not quoting the function, and using function() { this.count(); }

Using object.count() through Firebug works fine.

Yes that is a tricky problem. The issue is when the interval function is called, "this" is the timer object, not your object. So you are trying to call the method setInterval.count() essentially.

One solution is to use javascript closure. The way that would work is by saving the "this" value in a variable inside the constructor function. Then use that variable in the function like this.


timer.prototype.start = function() {
this.startBtn.disabled=1;
this.stopBtn.disabled=0;
this.resetBtn.disabled=0;
var thisTimerObj = this;
this.intervalId = setInterval(
function () { thisTimerObj.count(); },
1000);
);
};


What happens here is the variable thisTimerObj is saved with the timer object being instantiated. And since the anonymous function is defined within the constructor, a copy of this local variable is saved so it can be used in the function.

Hopefully that works for you. Closure is a difficult thing to describe.

david_kw

My revised code:

function timer(sm,ss,a,m,p) {
this.startMins=this.mins=sm;
this.startSecs=this.secs=ss;
this.action=a;
this.msg = m;
this.prefix = p;
this.stopBtn = document.getElementById(this.prefix + "stop");
this.startBtn = document.getElementById(this.prefix + "start");
this.resetBtn = document.getElementById(this.prefix + "reset");
this.display = document.getElementById(this.prefix + "display");
this.intervalId;
this.benchmark = 0;
this.loops = 0;
}

timer.prototype.count = function() {
eval(this.secs += this.action);
if (this.secs >= 60) {
this.secs = 0;
this.mins++;
}
if (this.secs < 0) {
if (this.mins > 0) {
this.secs = 59;
this.mins--;
}
else {
thisTimer = this;
alert(thisTimer.msg);
this.stop();
}
}
this.displayOutput();
}

timer.prototype.start = function() {
this.displayOutput();
this.startBtn.disabled=1;
this.stopBtn.disabled=0;
this.resetBtn.disabled=0;
var thisTimer = this;
this.intervalId = setInterval(function() {thisTimer.count()},1000);
}

timer.prototype.stop = function() {
thisTimer = this;
clearInterval(thisTimer.intervalId);
this.startBtn.disabled=0;
this.stopBtn.disabled=1;
}

timer.prototype.reset = function() {
this.stop();
this.resetBtn.disabled=1;
this.mins = this.startMins;
this.secs = this.startSecs;
this.displayOutput();
}

timer.prototype.displayOutput = function() {
this.display.innerHTML = this.mins+":"+formatTime(this.secs);
}

function formatTime(i) {
if (i<10)
{i = "0" + i}
return i
}

I'm sure I used a lot of closures where they aren't needed. Please, again, look over my code and point out any errors or inefficiencies.

I also have some pages that will be using this script that don't have buttons; instead, the operation of the calculator is done through other scripts. What is the best way of changing my code so that buttons aren't always required?

function timer(sm,ss,a,m,p) {
this.startMins=this.mins=sm;
this.startSecs=this.secs=ss;
this.action=a;
this.msg = m;
this.prefix = p;
this.stopBtn = document.getElementById(this.prefix + "stop");
this.startBtn = document.getElementById(this.prefix + "start");
this.resetBtn = document.getElementById(this.prefix + "reset");
this.display = document.getElementById(this.prefix + "display");
this.intervalId;
this.benchmark = 0;
this.loops = 0;
}

timer.prototype.count = function() {
// eval(this.secs += this.action);
this.secs += this.action;
// not sure whey you want the eval around it. It doesn't seem to do anything
if (this.secs >= 60) {
this.secs = 0;
this.mins++;
}
if (this.secs < 0) {
if (this.mins > 0) {
this.secs = 59;
this.mins--;
}
else {
// thisTimer = this;
// alert(thisTimer.msg);
alert(this.msg);
// don't need closure here because the code is executed "now" and not later so 'this' is fine
this.stop();
}
}
this.displayOutput();
}

timer.prototype.start = function() {
this.displayOutput();
this.startBtn.disabled=1;
this.stopBtn.disabled=0;
this.resetBtn.disabled=0;
var thisTimer = this;
this.intervalId = setInterval(function() {thisTimer.count()},1000);
// looks good
}

timer.prototype.stop = function() {
// thisTimer = this;
// clearInterval(thisTimer.intervalId);
clearInterval(this.intervalId);
// again this function happens right away so closure isn't needed
this.startBtn.disabled=0;
this.stopBtn.disabled=1;
}

timer.prototype.reset = function() {
this.stop();
this.resetBtn.disabled=1;
this.mins = this.startMins;
this.secs = this.startSecs;
this.displayOutput();
}

timer.prototype.displayOutput = function() {
this.display.innerHTML = this.mins+":"+formatTime(this.secs);
}

function formatTime(i) {
if (i<10)
{i = "0" + i}
return i
}


Those are the bits I noticed on my glance through. One other thing is that you forgot the "var" in front of your closure variable for a couple of the ones that weren't needed. You remembered it on the one where it was needed. If you don't put the "var" in then the variable will be global and that could mess you up.

All my changes should be in red.

Hope that helps.

david_kw

Thank you for the help. Any ideas on making the start/stop/reset buttons optional (in terms of disabling them) but not required?

I suppose instead of disabling the button you could keep the state in the object.

like in the constructor put


this.playing = false;
this.atBeginning = true;


then in each function do the appropriate checks like


timer.prototype.start = function() {
if (this.playing) {
return;
}
this.displayOutput();
this.playing = true;
this.atBeginning = false;
var thisTimer = this;
this.intervalId = setInterval(function() {thisTimer.count()},1000);
}


Something like that. So the object will keep its own start instead of counting on the buttons to keep the state for it.

david_kw