25 + 5 Clock - All Tests Pass but One

JavaScript
1

Hi All,

I’ve been working on this thing for over a week and finally got all the tests to pass except one. It keeps saying that the “session-label” element is not switching from “Break” to “Session” and vice versa when the timer reaches 00:00. However, when I run the timer it does indeed seem to switch at the correct time. I’ve been looking at my code and playing around with it for awhile but can’t seem to get this test to pass. Any ideas? What am I missing here?

2

Lower the setInterval delay to something like 250 so you do not have to wait so long.

  • Set both timers to 1 minute.

  • Let it run until it switches to the break time, then reset.

  • Set both timers back to 1 minute again.

  • Let it run until it switches to the break time, what do you see?

The DOM should reflect the internal state of the app. The app should not need to read the DOM to know what state it is in.

There are certain types of DOM manipulation where we use the DOM to look for app state, like when we use HTML attributes. But in general, you should avoid using the DOM to store the state of the app, nor should you rely on reading from it to know what state the app is in when at all possible.

You have some scary type conversions in your code. Switching the value type of variables depending on what state your app is in is just asking for trouble (see globalSeconds). You can convert types inline as needed, but you should not reassign the converted values back.

This is fine

const number = 123;
number.toString().length;

This is not

let number = 123;
number = number.toString();
number.length;
3

Thank you @lasjorg ! All tests are now finally passing! I did what you said, and figured out that the reset button needed to switch the sessionBreak variable flag back to 0 when it was clicked, so that the code in my interval would know to switch the timer-label element inner HTML from “Break” back to “Session”. After making that simple change all tests are passing now.

I changed the portion of my code that triggers switching from Session to Break so that it is not dependent on checking the state of the DOM to know when to switch from Break to Session. See below.

//When the countdown reaches 00:00, change from session to break minutes.
      if (intervalMinutes === "0-1" && globalSeconds === 59 && sessionBreak === 0) {
        globalBreakMinutes < 10 ? globalBreakMinutes = "0" + globalBreakMinutes : null;
        sessionBreak = 1;
        intervalMinutes = globalBreakMinutes;
        globalSeconds = "00";
        document.getElementById("timer-label").innerHTML = "Break";
        document.getElementById("beep").play();
      }
       else if (intervalMinutes === "0-1" && globalSeconds === 59 && sessionBreak === 1) {
        globalSessionMinutes < 10 ? globalSessionMinutes = "0" + globalSessionMinutes : null;
        sessionBreak = 0;
        intervalMinutes = globalSessionMinutes;
        globalSeconds = "00";
        document.getElementById("timer-label").innerHTML = "Session";
        document.getElementById("beep").play();
      }

One of the last things I want to do is fix a bug where if you press the start/stop button more than once per second, it creates a second interval that runs simultaneously and messes everything up. Maybe the setTimeout() method would be good to use for that?

As far as the scary conversions you mentioned, such as globalSeconds, could you give me a few examples in the code of what exactly I should change? I’m not 100% clear what parts of the code are OK and what should be changed.

Thanks again for your detailed responses, they have helped a lot!

4

You can clear the interval at the top of the start function if interval contains a value.

const start = () => {
  interval && clearInterval(interval);
  // rest of code
}

You should never reassign the variable value type. It is a bad idea and will lead to bugs and make the code harder to reason about. When is it a string? When is it a number? Why do we have to follow the logic all the way through the code to know what type the variable contains at any given point in the code?

If you initialized it as a String, it is a String, not a Number. It should not suddenly turn into a Number.

You can convert the type without reassigning and you can have two different versions of the variable (named appropriately). But the same variable should never change its type.

1 Like
5

That makes total sense when you explain it that way. I’ll try and rework the code so that globalSeconds and sessionMinutes aren’t getting changed back and forth from number to string. What if, instead of saying…

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

I rewrite it to something like this? Would that work?

if (globalSeconds < 10 && sessionMinutes > 9) {document.getElementById("time-left".innerHTML = `${sessionMinutes}:0${globalSeconds}`};

else if (globalSeconds > 9 && sessionMinutes < 10) {document.getElementById("time-left".innerHTML = `0${sessionMinutes}:${globalSeconds}`};

else if (globalSeconds < 10 && sessionMinutes < 10) {document.getElementById("time-left".innerHTML = `0${sessionMinutes}:0${globalSeconds}`};

else {document.getElementById("time-left".innerHTML = `${sessionMinutes}:${globalSeconds}`}```
6

I would suggest you test it. But yes, that code is much easier to reason about and is an improvement no matter what.

1 Like