Help with Pomodoro Clock

Hi everyone,
I have been researching/trying to figure out how to solve my pomodoro clock issue and I can’t spend anymore time on this :stuck_out_tongue:
This is still a work in progress so don’t judge me too harshly.
The issue I’m having is that once the counter starts it keeps going past zero into negative numbers and I have not been able to correct this.
If someone could take a look and help me figure this out it would be a big help as I have spend far too long on this (and I probably should know how to do it but I’m just now getting back into coding after a long break due to work stuff).
Here’s a link to my codepen:


THanks!

OK,

I have some questions about the code that you have:

function startTimer() {
  var minutes = 25, seconds = 60;
  $clock.text($timer.text());
  minutes = Number($timer.text())-1;

  timer = setInterval(function() {
    seconds--;
    var ended;
    if (ended = !minutes && !seconds) {
      stopTimer();
    };
    $clock.text(minutes + ':' + seconds);
  }, 1000);
  if (seconds===0) {
    minutes-1;
    seconds===60
    timer;
  }
}

function stopTimer() {
  clearTimeout(window.timer);
  $clock.text($timer.text());
} 

What is the variable ended doing? It never gets used?

What does the line minutes-1; do? It evaluates an expression but does nothing with it. I assume you mean minutes-=1;

What does the line seconds===60 do? It evaluates an expression but does nothing with it. I assume that you mean seconds = 0;.

What does the line timer; do? It evaluates an expression but does nothing with it.

And please indent properly - it makes everyone’s life (including yours) easier.

So, the problem I see is what I listed above, and some of the logic. Within the timer, you check if minutes and seconds are zero and if they are, call stopTimer - OK, makes sense. You also have logic to check if seconds is 0. But that’s outside your timer. When is it going to be checked? Only once when startTimer is called. That logic needs to be inside the timer and it needs to be an else if to the first if statement. You need to check if they are both 0 and then call stopTimer and only do that, then, else, see if seconds is 0 and toggle both.

Is that clear? I don’t want to actually write out the code for you, but when I make those changes, it works for me.

So, fix the minutes-1; and seconds===60 lines. Put the if (seconds===0) { line in it’s proper place, as an else if on of the timer’s if statement.

I also don’t think you need the variable ended (at least not as the code stands now) and the statement timer; does nothing.

I also don’t like implicitly global variables. You use the variable timer inside startTimer() but it is never declared. This allows it to be declared globally. I think this is sloppy. If you’re going to use a global variable (which I did here too), declare it globally. Then your statement in stopTimer() can just be clearTimeout(window.timer);

Thanks a lot!!! I was getting frustrated, so I got kind of sloppy adding unnecessary code trying to experiment. When you explained it, I realized how bad my code was :stuck_out_tongue:I guess you learn by making mistakes. Now to finish it up!!! :slight_smile:

We’ve all been there. But like my old pizza parlor manager said, CAYG - clean as you go. If you keep things tidy as you go, you won’t get walloped with a big clean at the end and things tend to go smoother anyway. I know, sometimes it’s easier said than done.

But if I run into a stumbling block like you did, the first thing I do is put in a bunch of console.log statements to make sure that everything is behaving the way I want. And if that doesn’t solve it, I start going line by line and reading everything out loud to make sure it does what I want. And I start checking the logic.

Yes, we learn by making mistakes. I just try not to make the same one a seventh or eighth time. :wink:

Good luck.

Hey, just wanted to thank you for asking for help. It helped me realize a bug in mine that I didn’t know I had.

Glad it helped! :slight_smile: