3

I would like to create a simple timer in Javascript that counts down from a given time until it hits 0. I found this tutorial which worked perfectly. My problem is that I need to place multiple timers on the same page. This tutorial obviously won't do that because it uses global variables (I'm new to JS/Programming so I might not be using the right terms). I tried to re-create the same thing only creating each timer as it's own Object so that they don't interfere with eachother. This is what I have.

function taskTimer(name, startTime) {
  this.timer = name;
  this.totalSeconds = startTime;
  this.tick = function() {
    if (this.totalSeconds <= 0) {
      return;
    }
    this.totalSeconds -= 1;
    this.updateTimer();
    // window.setTimeout("this.tick()", 1000);
  };
  this.updateTimer = function(){
    this.seconds = this.totalSeconds;

    this.hours = Math.floor(this.seconds / 3600);
    this.seconds -= this.hours * (3600);

    this.minutes = Math.floor(this.seconds / 60);
    this.seconds -= this.minutes * (60);

    this.timeString = this.leadingZero(this.hours) + ":" + this.leadingZero(this.minutes) + ":" + this.leadingZero(this.seconds);
    return this.timeString;
  };
  this.leadingZero = function(time){
    return (time < 10) ? "0" + time : + time;
  };
}
var testTimer = new taskTimer("timer", 30);
testTimer.tick();

I created one at the end there. Running testTimer.updateTimer(); returns 00:00:30 which is correct, but running testTimer.tick(); returns no value. There is obviously something wrong with that part of the code I just can't figure it out.

Shmiddty
  • 13,847
  • 1
  • 35
  • 52
SMDeConto
  • 35
  • 1
  • 4
  • how can you tell if this is working or not? you're not painting any of the timer values anywhere. – jbabey Jan 03 '13 at 21:04
  • because the updateTimer method works, so calling the tick method should return the same value because it calls the updateTimer method inside of it. – SMDeConto Jan 03 '13 at 21:19
  • "`testTimer.tick();` returns no value" Is that the problem you are trying to solve? If so, that function doesn't return anything, ever. You need a `return` statement in there. – Alex Wayne Jan 03 '13 at 21:39
  • But testTimer.tick(); calls the updateTimer(); method which does return a value. Shouldn't that mean testTimer.tick(); would return that same value? – SMDeConto Jan 03 '13 at 21:47
  • @SMDeConto updateTimer returns a value to tick, but tick doesn't return it to you. – Christophe Jan 03 '13 at 21:52
  • @SMDeConto `this.updateTimer();` at the end of `this.tick` should be `return this.updateTimer();` – jbabey Jan 03 '13 at 22:16
  • @SMDeConto I think is late but I was posted my version anyway. – HMarioD Jan 04 '13 at 01:26

3 Answers3

4

You've got a few problems.

  1. You're calling updateTimer() inside of your tick method, so it won't ever reach outside of there unless you return it.
  2. With your current setup, you'd have to call tick manually every time you wanted to update the clock, and if you don't do that precisely every one second the timer will be inaccurate.
  3. To go with #2, you shouldn't decrement totalSeconds like you are because it isn't guaranteed that it will be exactly one second between triggers of your timeout. Use dates instead.

Here's what I would do: http://jsfiddle.net/R4hnE/3/

// I added optional callbacks. This could be setup better, but the details of that are negligible. 
function TaskTimer(name, durationInSeconds, onEnd, onTick) {
    var endTime,
        self = this, // store a reference to this since the context of window.setTimeout is always window
        running = false;
    this.name = name;
    this.totalSeconds = durationInSeconds;

    var go = (function tick() {
        var now = new Date().getTime();
        if (now >= endTime) {
            if (typeof onEnd === "function") onEnd.call(self);
            return;
        }
        self.totalSeconds = Math.round((endTime - now) / 1000); // update totalSeconds placeholder
        if (typeof onTick === "function") onTick.call(self);
        window.setTimeout(tick, 1000 / 12); // you can increase the denominator for greater accuracy. 
    });

    // this is an instance method to start the timer
    this.start = function() {
        if (running) return; // prevent multiple calls to start

        running = true;
        endTime = new Date().getTime() + durationInSeconds * 1000; // this is when the timer should be done (with current functionality. If you want the ability to pause the timer, the logic would need to be updated)
        go();
    };
}
// no reason to make this an instance method :)
TaskTimer.prototype.toTimeString = function() {
    var hrs = Math.floor(this.totalSeconds / 60 / 60),
        min = Math.floor(this.totalSeconds / 60 - hrs * 60),
        sec = this.totalSeconds % 60;

    return [hrs.padLeft("0", 2), min.padLeft("0", 2), sec.padLeft("0", 2)].join(" : ");
};

var task = new TaskTimer("task1", 30, function() {
    document.body.innerHTML = this.toTimeString();
    alert('done');
}, function() {
    document.body.innerHTML = this.toTimeString();
});
Shmiddty
  • 13,847
  • 1
  • 35
  • 52
  • Also note that convention would have classes start with Uppercase letters. – Shmiddty Jan 03 '13 at 21:58
  • Ill work on understand this lol. Also, I am trying to have multiple timers running at the same time. – SMDeConto Jan 03 '13 at 22:42
  • Fantastic. I don't understand it all yet but I'll get it :) Thank you! – SMDeConto Jan 03 '13 at 22:53
  • @SMDeConto do you have any specific questions? – Shmiddty Jan 03 '13 at 22:55
  • I will I'm sure but so far I'm having fun figuring out some of the methods/syntaxes you've used. I just started school and am trying to do some independent studying on my own during break so I created this project for myself. I think it's helpful for me to read someone elses code. I'll let you know if there's anything I can't figure out. Thanks! – SMDeConto Jan 03 '13 at 23:05
0

I always have problems with this and in one instance, within the function, I had to redefine it:

this.tick = function() {
  self=this;
  if (self.totalSeconds <= 0) {
    return;
  }
  self.totalSeconds -= 1;
  self.updateTimer();
   // window.setTimeout("self.tick()", 1000);
};

Here is another post about that: var self = this?

Community
  • 1
  • 1
Robot Woods
  • 5,677
  • 2
  • 21
  • 30
  • actually you don't need this in code you provided. but when you use callback, like in setTimeout or something else - you do need it (seems you wanted to highlight this with commenting it) – llamerr Jan 03 '13 at 21:02
  • 1
    Why quote the call to tick? I think this is a bit cleaner: setTimeout(self.tick, 1000); – psema4 Jan 03 '13 at 21:02
  • 2
    You're declaring self as a global. tsk tsk. – Shmiddty Jan 03 '13 at 21:04
  • I only commented it because I was testing the tick method and didn't want it to run again. This didn't work for me either, it just froze up. – SMDeConto Jan 03 '13 at 21:22
0

My version, you can see at: http://fiddle.jshell.net/hmariod/N7haK/4/

var tmArray = new Array();
var timerRef, timerId=0;


function newTimer(){
    try{
        if(tmArray.length > 4) throw "Too much timers";
        var countDown = parseInt(document.getElementById("tCountown").value,10);
        if(isNaN(countDown)) throw "tCountown is NaN";
        var tmName = document.getElementById("tName").value;
        var nt = new taskTimer(++timerId, tmName, countDown);
        createTmElement(timerId, tmName);
        tmArray.push(nt);
        if(!timerRef) timerRef = setInterval(timerFn, 1000);
        showTimers();
    }catch(er){
        alert("newTimer:" + er);
    }
}

function taskTimer(id, name, tCountown) {
    this.id = id;
    this.tName = name;
    this.tCountown = tCountown;
}


function timerFn(){
    try{
        var i;
        killTimer = true;
        for(i = 0; i < tmArray.length; i++){
           tmArray[i].tCountown--;
            if(tmArray[i].tCountown < 0){
                tmArray[i].tCountown = 0;
            }else{
                killTimer = false;
            }
        }
        if(killTimer) clearInterval(timerRef);
        showTimers();
    }catch(er){
        clearInterval(timerRef);
        aler("timerFn: " + er);
    }
}
HMarioD
  • 842
  • 10
  • 18