3

I'm coding a simple countdown timer in javascript, with play and pause buttons. It works fine when I start it, but if I pause/play a few times the timer values changes randomly, I can't figure out why.

Here's my code : 

var minutesleft = 60;
var secondsleft = 0;
var millisecondsleft = 0;
var bool = true;
var paused = false;
var end;
var now;

function pause(){
    paused = true;
}

function stop(){
    end = now;
    bool = true;
}

function cd(){
    if (bool) {
        end = new Date();
        end.setMinutes(end.getMinutes()+minutesleft);
        end.setSeconds(end.getSeconds()+secondsleft);
        end.setMilliseconds(end.getMilliseconds()+millisecondsleft);
        bool = false;
    }
now = new Date();
diff = end - now;
diff = new Date(diff);
var msec = diff.getMilliseconds();
var sec = diff.getSeconds();
var min = diff.getMinutes();
if (min < 10){
    min = "0" + min;
}
if (sec < 10){
    sec = "0" + sec;
}
if(msec < 10){
    msec = "00" +msec;
}
else if(msec < 100){
    msec = "0" +msec;
}
if(now >= end){
    clearTimeout(timerID);
    document.getElementById("cdtime").innerHTML = 'POLICE IS HERE';
}
else{
document.getElementById("cdtime").innerHTML = min + ":" + sec + ":" + msec;
}
if (paused == false){
    timerID = setTimeout("cd()", 10);
}
else {
    bool = true;
    minutesleft = min;
    secondsleft = sec;
    millisecondsleft = msec;
}
paused = false;
}

Here's a fiddle : https://jsfiddle.net/cw3s5124/

Thanks in advance

elachere
  • 595
  • 2
  • 8
  • 20
  • What is your bool variable actually doing? Consider naming the variable something descriptive to help other developers debug your code at a later time... or on stack-o :) – Chizzle Dec 09 '15 at 16:39
  • Sorry, i'ts just a boolean that helps me know if this is the first time the function is called or the first time since I paused – elachere Dec 09 '15 at 17:11
  • I think I found the issue which was in the string concatenation, adding 0s – Chizzle Dec 09 '15 at 20:06

2 Answers2

2

The main issue with the jumping numbers was due to string concatenation and then using that string in the date functions. Parts of the code such as this:

if (min < 10){
    min = "0" + min;
}

An example proving why this is happening

var   end1 = new Date(); 
var   minutesleft1 = 9;
console.log("Minutes to assign to current time");
console.log(end1.getMinutes() + minutesleft1); // Logs expected amount of minutes to calculate
end1.setMinutes(end1.getMinutes() + minutesleft1);

var   end2 = new Date(); 
var   minutesleft2 = "09";
console.log("Wrong Minutes to assign to current time");
console.log(end2.getMinutes() + minutesleft2); // Logs big number
end2.setMinutes(end2.getMinutes() + minutesleft2);

console.log(end1); // Logs expected result
console.log(end2); // Logs a time a day or so in the future

■

Explanation: If you were to set your start timer to 9 in the minutes var, it would immediately need to be padded with a 0 for display. Since you are now setting the minutesleft variable to "09" you are effectively doing 9 + "09" when you call end.setMinutes(end.getMinutes()+minutesleft);

Once you concatenate a string "0" to the number, you are effectively trying to do numeric operations with a string, which is causing unexpected results. This wasn't happening at the start, because your if statements weren't running yet and thus the min, sec, and msec vars were still numeric. You should only pad the numbers when displaying them, not in the calculations themselves.

I replaced the string concatenation with a padding function borrowed from this post.

I also added a play function to reset your pause parameter. This makes the code more readable and easier for me to understand. Also, change it to paused at the start because the timer isn't running yet.

var minutesleft = 10;
var secondsleft = 05; // This would normally just be returned as 5, that's why we need the pad function for display
var millisecondsleft = 0;
var firstCall = true;
var paused = true;
var end;
var now;

function pause() {
  paused = true;
}

function play() {
  if (paused === false) // Shorcut out because we are already running
    return;

  paused = false;
  cd();
}

function stop() {
  end = now;
  paused = true;
  cd();
}

document.addEventListener("DOMContentLoaded", function(event) {
  document.getElementById("cdtime").innerHTML = pad(minutesleft, 2) + ":" + pad(secondsleft, 2) + ":" + pad(millisecondsleft, 2);
});

function cd() {
  if (firstCall) {
    end = new Date();
    end.setMinutes(end.getMinutes() + minutesleft);
    end.setSeconds(end.getSeconds() + secondsleft);
    end.setMilliseconds(end.getMilliseconds() + millisecondsleft);
    firstCall = false;
  }
  now = new Date();
  diff = end - now;
  diff = new Date(diff);
  var msec = diff.getMilliseconds();
  var sec = diff.getSeconds();
  var min = diff.getMinutes();

  if (now >= end) {
    clearTimeout(timerID);
    document.getElementById("cdtime").innerHTML = 'POLICE IS HERE';
  } else {
    document.getElementById("cdtime").innerHTML = pad(min, 2) + ":" + pad(sec, 2) + ":" + pad(msec, 2);
  }
  if (paused === false) {
    timerID = setTimeout("cd()", 10);
  } else {
    bool = true;
    minutesleft = min;
    secondsleft = sec;
    millisecondsleft = msec;
  }
}

function pad(n, width, z) {
  z = z || '0';
  n = n + '';
  return n.length >= width ? n : new Array(width - n.length + 1).join(z) + n;
}
<body>
  <div id='timer'>
    <button class='playerButtons' id='playT' type='submit' onclick='play()'>Play</button>
    <button class='playerButtons' id='pauseT' type=' submit' onclick='pause()'>Pause</button>
    <button class='playerButtons' id='stopT' type='submit' onclick='stop()'>Stop</button>
    <div id='cdtime'></div>
  </div>
</body>
Community
  • 1
  • 1
Chizzle
  • 1,707
  • 1
  • 17
  • 26
  • Thank's man it works ! Just had to set firstCall true in the play function after cd() call so that after pausing the timer continues where it was paused. Thank you a lot again ! – elachere Dec 10 '15 at 09:23
  • I'm new in web development, and I got a little question now : is there a way to set the value of a variable in the script from the html ? What I want is, for example, a button in html that add 2 min to the counter when clicked, is that possible ? – elachere Dec 10 '15 at 09:32
  • I just realized that's what I'm doing when I pause or play, I set the value of paused. So I tried a function addTime() {minutesleft += 2} that I call on a button click, but it doesn't work, why ? – elachere Dec 10 '15 at 10:28
  • Please check my new edit, proving why the number jumping was happening :) – Chizzle Dec 10 '15 at 17:12
  • Thank you very much, very instructive :) – elachere Dec 11 '15 at 20:37
1

You reset paused and bool in the wrong spot. If you look at the JsFiddle, you can see what needs to be removed

var minutesleft = 60;
var secondsleft = 0;
var millisecondsleft = 0;
var bool = true;
var paused = true;
var end;
var now;

function pause(){
    paused = true;
      //bool = false; //shouldn't be here (move to stop function)
}
function play(){
   if ( paused === true){
      paused = false;
      bool = true;
      cd();
    }
}

function stop(){
    end = now;
    bool = false; //add here
}

document.addEventListener("DOMContentLoaded", function(event) {
 document.getElementById("cdtime").innerHTML = minutesleft + ":" + secondsleft + "0:" + millisecondsleft + "0";
});
function cd(){
    if (bool) {
        end = new Date();
        end.setMinutes(end.getMinutes()+minutesleft);
        end.setSeconds(end.getSeconds()+secondsleft);
        end.setMilliseconds(end.getMilliseconds()+millisecondsleft);
        bool = false;
    }
    now = new Date();
    diff = end - now;
    diff = new Date(diff);
    var msec = diff.getMilliseconds();
    var sec = diff.getSeconds();
    var min = diff.getMinutes();
    if (min < 10){
        min = "0" + min;
    }
    if (sec < 10){
        sec = "0" + sec;
    }
    if(msec < 10){
        msec = "00" +msec;
    }
    else if(msec < 100){
        msec = "0" +msec;
    }
    if(now >= end){
        clearTimeout(timerID);
        document.getElementById("cdtime").innerHTML = 'POLICE IS HERE';
    }
    else{
    document.getElementById("cdtime").innerHTML = min + ":" + sec + ":" + msec;
    }
    if (paused === false){
        timerID = setTimeout("cd()", 10);
    }
    else {
        bool = true;
        minutesleft = min;
        secondsleft = sec;
        millisecondsleft = msec;
    }
    //paused = false; //shouldn't be here
}
<body>
<div id='timer'>
  <button class='playerButtons' id='playT' type='submit' onclick='play()'>Play</button>
  <button class='playerButtons' id='pauseT' type=' submit'onclick='pause()'>Pause</button>
  <button class='playerButtons' id='stopT' type='submit' onclick='stop()'>Stop</button>
<div id='cdtime'></div>
</div>
</body>

https://jsfiddle.net/cw3s5124/27/

Joe Packer
  • 525
  • 3
  • 15
  • Thank's for your answer, I was pretty that was about booleans, but there's still the same bug with your code (less frequently, but still) – elachere Dec 09 '15 at 17:20
  • Could you go into more detail about the issue that you are seeing? – Joe Packer Dec 09 '15 at 18:37
  • When using play/pause, at some point the timer values changes randomly, just try it, play and pause a few times you'll see. I can't explain it, that's really strange. – elachere Dec 09 '15 at 18:43