Neither If Nor Else are Executing For a Class in Javascript

Question

The Stopwatch is not working. I have tried to place console.logs everywhere to see if something is not being executed, and all I can find is the if statement in the start method is not working. Implemented an else with it to see if the statement had condition issues, but the else would also fail to activate. All I can predict is that there is an un-thrown error in the if statement. Perhaps it is something else, but I have been trying to fix it for days and cannot figure it out.

class StopWatch {
  constructor() {
    this.sec = 0
    this.min = 0
    this.hour = 0
    this.startButton = document.getElementById("start")
    this.stopButton = document.getElementById("stop")
    this.resetButton = document.getElementById("reset")
    this.time = document.getElementById("timer")
    this.stopped = true
  }
  start() {
    if (this.stopped === true) {
      this.stopped = false
      this.count
    }
  }
  stop() {
    if (this.stopped === false) {
      this.stopped = true
    }
  }
  reset() {
    this.stopped = true
    this.time.innerHTML = '00:00:00'
  }
  count() {
    if (this.stopped === false) {
      this.sec = parseInt(this.sec)
      this.min = parseInt(this.min)
      this.hour = parseInt(this.hour)
      this.sec += 1
      if (this.sec == 60) {
        this.sec = 0
        this.min = 1
      }
      if (this.min == 60) {
        this.min = 0
        this.hour = 1
      }
      if (this.sec <= 10) {
        this.sec = `0${this.sec}`
      }
      if (this.min <= 10) {
        this.min = `0${this.min}`
      }
      if (this.hour <= 10) {
        this.hour = `0${this.hour}`
      }
      this.time.innerHTML = `${this.hour}:${this.min}:${this.sec}`
      setTimeout(this.count, 1000)
    }
  }
}

const watch = new StopWatch()
watch.startButton.onclick = watch.start
watch.stopButton.onclick = watch.stop
watch.resetButton.onclick = watch.reset

<!DOCTYPE html>
<html lang="en">

<head>
  <meta charset="UTF-8">
  <meta http-equiv="X-UA-Compatible" content="IE=edge">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>Document</title>
  <script src="game.js" defer></script>
  <style></style>
</head>

<body>
  <h1>STOPWATCH</h1>
  <h2>Vanilla Javascript Stopwatch</h2>
  <span id="timer">00:00:00</span>
  <button id="start">Start</button>
  <button id="stop">Stop</button>
  <button id="reset">Reset</button>
</body>

</html>

Use: watch.startButton.onclick = watch.start.bind(watch); watch.stopButton.onclick = watch.stop.bind(watch); watch.resetButton.onclick = watch.reset.bind(watch); — amishra, Apr 03 '22 at 23:13

score 0 · Answer 1 · answered Apr 03 '22 at 22:42

This is most likely caused by the fact that the value of this depends on how the function is called. In this case the value will be the element to which the event was attached.

For example, here you can see this becomes the button element:

class Logger {
  foo = "bar";

  logThis() {
    console.log(this);
  }
}

const logger = new Logger();

document.getElementById("my-button").onclick = logger.logThis;

<button id="my-button">Click me!</button>

If you want to preserve the value of this you will have to bind it's value so it is preserved.

class Logger {
  foo = "bar";

  logThis() {
    console.log(this);
  }
}

const logger = new Logger();

document.getElementById("my-button").onclick = logger.logThis.bind(logger);

<button id="my-button">Click me!</button>

score 0 · Answer 2 · edited Apr 04 '22 at 06:15

Use arrow function as the this is not what you expect it to be in your code. Log this in your functions in your and my version to see what you were referencing and what this refers to in my version. I suggest you also have a look at the MDN docs to read more about this. To resolve your issue you can use arrow functions.

There are four more issues in your code though:

When you reset time you do not reset the minutes, seconds and hours
You should wait for the DOM to be loaded before calling getElementById(). See MDN docs.
Your if condition is using <= 10 which will result in three digits being displayed when 10 is displayed. Change that to < 10.
Why not attach the event handlers in the constructor?

Here your code with above issues removed except for setting the event handlers in the constructor to show you how you could use arrow function to make your code work.

class StopWatch {
  constructor() {
    this.sec = 0;
    this.min = 0;
    this.hour = 0;
    this.stopped = true;
    this.startButton = document.getElementById("start");
    this.stopButton = document.getElementById("stop");
    this.resetButton = document.getElementById("reset");
    this.time = document.getElementById("timer");
  }

  start() {
    if (this.stopped === true) {
      this.stopped = false;
      this.count();
    }
  }

  stop() {
    if (this.stopped === false) {
      this.stopped = true;
    }
  }

  reset() {
    this.stopped = true;
    this.time.innerHTML = "00:00:00";
    // bug here => you need to reset the rest as well
    this.sec = 0;
    this.min = 0;
    this.hour = 0;
  }

  count() {
    if (this.stopped === false) {
      this.sec += 1;

      if (this.sec == 60) {
        this.sec = 0;
        this.min += 1;
      }

      if (this.min == 60) {
        this.min = 0;
        this.hour += 1;
      }
      
      // display number in atleast 2 digits (add 0 at start)
      this.time.innerHTML = `${this.hour.toString().padStart(2,'0')}:${this.min.toString().padStart(2,'0')}:${this.sec.toString().padStart(2,'0')}`;
      setTimeout(() => this.count(), 1000);
    }
  }
}

// you should generally wait until the DOM is loaded otherwise your getElementById calls might not work
window.addEventListener("DOMContentLoaded", event => {
  const watch = new StopWatch();
  // use arrow funcitons
  watch.startButton.onclick = () => watch.start();
  watch.stopButton.onclick = () => watch.stop();
  watch.resetButton.onclick = () => watch.reset();
})

<h1>STOPWATCH</h1>
<h2>Vanilla Javascript Stopwatch</h2>
<span id="timer">00:00:00</span>
<button id="start">Start</button>
<button id="stop">Stop</button>
<button id="reset">Reset</button>

Another thing you could probably change is to use min, sec and hour as numbers and only format them as you need them in the time <span>. This way you don't need to unnecessarily parse min, sec and hour every second. You could be formatting your numbers using this.min.toString().padStart(2, "0") when they are numbers. This way you will also get rid of some of your if conditions.

Neither If Nor Else are Executing For a Class in Javascript

2 Answers2