gridproquo
gridproquo

Reputation: 129

Alert window popping up multiple times when timer finishes

I'm working on a simple timer using JS. When the timer hits 0, I want "time up!" to pop up.

$(document).ready(function() {
    updateClock();
    var timeInterval = setInterval(updateClock(), 1000);
});

var ms = 5000;

function updateClock() {
    ms -= 1000;
    var minutes = Math.floor(ms / 1000 / 60);
    var seconds = Math.floor((ms / 1000) % 60);
    $('#minutes').html(minutes);
    $('#seconds').html(seconds);
    if (ms <= 0) {
        alert('Time is up!');
        clearInterval(timeInterval);
    };
}

Right now, the alert pops up multiple times. I suspect it's because I'm using clearInterval() incorrectly—when I open up the developer console, it says:

"Uncaught ReferenceError: timeInterval is not defined at updateClock."

However, I'm not sure what to change to make it function correctly.

Upvotes: 3

Views: 414

Answers (2)

gaetanoM
gaetanoM

Reputation: 42044

According to MDN doc:

var intervalID = window.setInterval(func, delay[, param1, param2, ...]);

var intervalID = window.setInterval(code, delay);

where:

func

      A function to be executed every delay milliseconds.

This means that if you call this function with first parameter like:

updateClock()

the updateClock function is executed and the result code is returned to the setInterval function. But, because such a function (i.e.: updateClock) returns undefined, the setInterval will have nothing to execute the next times.

For the other problem (i.e.: local variable) you may declare the variable "timeInterval" as a global function in this way:

window.timeInterval = setInterval(updateClock, 1000);

This will assure the value of the global variaable (i.e.: timeInterval) is available to the timer handler immediately.

The example:

$(document).ready(function() {
  window.timeInterval = setInterval(updateClock, 1000);
});

var ms = 10000;

function updateClock() {
  ms -= 1000;
  var minutes = Math.floor(ms / 1000 / 60);
  var seconds = Math.floor((ms / 1000) % 60);
  $('#minutes').text(minutes);
  $('#seconds').text(seconds);
  if (ms <= 0) {
    setTimeout(function() { // this to refresh before alert
      alert('Time is up!');
    }, 10);
    clearInterval(timeInterval);
  };
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>


<span id="minutes">0</span>:<span id="seconds">10</span>

You might also cache your min and sec dom elements instead to searches for them every time:

$(document).ready(function() {
  var minObj = $('#minutes');
  var secObj = $('#seconds')
  window.timeInterval = setInterval(function () {
    updateClock(minObj, secObj);
  }, 1000);
});

var ms = 10000;

function updateClock(minObj, secObj) {
  ms -= 1000;
  var minutes = Math.floor(ms / 1000 / 60);
  var seconds = Math.floor((ms / 1000) % 60);
  minObj.text(minutes);
  secObj.text(seconds);
  if (ms <= 0) {
    setTimeout(function() { // this to refresh before alert
      alert('Time is up!');
    }, 10);
    clearInterval(timeInterval);
  };
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>


<span id="minutes">0</span>:<span id="seconds">10</span>

Upvotes: 1

CodeLikeBeaker
CodeLikeBeaker

Reputation: 21312

Your variable timeInterval is inside $(document).ready(...)

Try this:

var timeInterval;

$(document).ready(function() {
    updateClock();
    timeInterval = setInterval(updateClock(), 1000);
});

var ms = 5000;

function updateClock() {
    ms -= 1000;
    var minutes = Math.floor(ms / 1000 / 60);
    var seconds = Math.floor((ms / 1000) % 60);
    $('#minutes').html(minutes);
    $('#seconds').html(seconds);
    if (ms <= 0) {
        alert('Time is up!');
        clearInterval(timeInterval);
    };
}

Upvotes: 1

Related Questions