Using setInterval() and clearInterval() in classes

Question

I have a problem understanding how to use setInterval() and clearInterval() in classes. I'm trying to build a timer that starts from 0 until i decide to stop it.

Up to now i managed to have a method that when you call it, the timer starts but when i try to pause it it just ignores my method and continues.

HTML

<!DOCTYPE html>
<html lang="en" xmlns="http://www.w3.org/1999/xhtml">
<head>
    <meta charset="utf-8" />
    <title></title>
</head>
  <body>
    <div class="container">
      <p id="timer">im here</p>
    </div>
    <button>Start/Stop Timer</button>
    <button>Reset Timer</button>
    <script src="script.js"></script>
  </body>
</html>

JS

class Timer {
  constructor(){
    this.isRunning = false;
    this.currTimer = 0;
    this.runTimer;
    var timer = document.getElementById('timer');
  }

  start(){
    this.isRunning = true;
    if (this.isRunning) {
      var currentTime = this.currTimer;
      this.runTimer = setInterval(function(){
        timer.innerHTML = ++currentTime;
      },1000)
    }
  }

  pause(){
    this.isRunning = false;
    if (this.isRunning = false) {
      clearInterval(this.runTimer)
    }
  }
}

var test = new Timer();
test.start();
test.pause();// put this after a few seconds

I'm also pretty sure i'm using "this" wrong. I just learned this new stuff and trying to build it.

`if (this.isRunning = false) {` — You made a typo. That's an assignment, not a comparison. — Quentin, May 31 '18 at 07:12
I've rolled back the edit that fixed it. Questions shouldn't be moving targets. :-) — T.J. Crowder, May 31 '18 at 07:16

T.J. Crowder · Accepted Answer · 2018-05-31T07:20:49.343

If you want to use it after the object is constructed, your timer local variable needs to be a property instead, so in the constructor, change

var timer = document.getElementById('timer');

to

this.timer = document.getElementById('timer');

Then in start, you need to use this.timer as well — and that means you want to use an arrow function, not a traditional function, for the setInterval callback so that the function closes over this. You probably want to use this.currTimer directly rather than copying it to a variable:

this.runTimer = setInterval(() => {
    this.timer.innerHTML = ++this.currTimer;
},1000);

The logic there also needs tweaking: You've just assigned true to this.isRunning, so there's no need to check it afterward. But really, you want to check it beforehand:

start(){
  if (!this.isRunning) {
    this.isRunning = true;
    this.runTimer = setInterval(() => {
      this.timer.innerHTML = ++this.currTimer;
    },1000);
  }
}

You also need to use == or ===, not =, when doing comparisons like this one:

if (this.isRunning = false) { // Needs == or ===

But the logic in pause has a problem: You've just set this.isRunning to false, so checking it on the next line, it'll always be false. Instead, check it before you assign to it. Also, use just if (flag) or if (!flag) rather than using == true or == false:

pause(){
  if (this.isRunning) {
    clearInterval(this.runTimer)
    this.isRunning = false;
  }
}

Seems to work with those changes:

class Timer {
    constructor() {
        this.isRunning = false;
        this.currTimer = 0;
        this.runTimer;
        this.timer = document.getElementById('timer');
    }

    start() {
        if (!this.isRunning) {
            this.isRunning = true;
            this.runTimer = setInterval(() => {
                this.timer.innerHTML = ++this.currTimer;
            }, 1000);
        }
    }

    pause() {
        if (this.isRunning) {
            clearInterval(this.runTimer);
            this.isRunning = false;
        }
    }
}

var test = new Timer();
test.start();
setTimeout(function() {
  test.pause(); // put this after a few seconds
}, 4000);

<div id="timer"></div>

I also strongly recommend using ; consistently (such as after the call to setInterval). JavaScript has automatic semicolon insertion (ASI) so it will act like you included semicolons in some places even if you haven't, but you can't rely on it unless you have a very thorough understanding of the ASI rules.

score 0 · Answer 2 · answered May 31 '18 at 07:11

0

In your condition, you're assigning instead of comparing.

This:

pause() {
  this.isRunning = false;
  if (this.isRunning = false) {
    clearInterval(this.runTimer)
  }
}

Should be:

pause() {
  this.isRunning = false;
  if (this.isRunning === false) {
    clearInterval(this.runTimer)
  }
}

Or, because the condition will always evaluate to true, just:

pause() {
  this.isRunning = false;

  clearInterval(this.runTimer)
}

answered May 31 '18 at 07:11

Robby Cornelissen

91,784
22
134
156

1

It should probably `if (this.isRunning) { clearInterval(this.runTimer); this.isRunning = false; }` but that would require fixes to the rest of the code. – melpomene May 31 '18 at 07:12

Using setInterval() and clearInterval() in classes

2 Answers2