London | 26-ITP-Jan | Carlos Abreu |Sprint 3 | Alarm Clock

[carlosyabreu]

Learners, PR Template

Self checklist

• I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
• My changes meet the requirements of the task
• I have tested my changes
• My changes follow the style guide

Changelist

Alarm clock app PR for Sprint 3 module

[cjyuan]

Why not declare currentTime as a local variable inside setAlarm()?

[carlosyabreu]

Sorry @cjyuan for the delay.
I've been busy and stressed working on last and final module (Moduel Data Flows) and the Project-TV-Show with a colleague and working in a collaborative way with Git and GitHub has been a hell to understand its workflow.
It's been challenging but rewarding to finally get a glimpse how it works in a shared environment.
As I'm almost done with Module Data Flows and Project TV Show (I'm at level 500 just to finish today later) I'm back to Module Data Group.

Going back to your query.
The reason currentTime is declared outside setAlarm() (at the module level) is that it needs to be shared and changed by multiple functions:
setAlarm() initializes currentTime and starts the interval that decrements it.
The interval callback inside setAlarm() updates currentTime every second.
updateTimeDisplay() reads currentTime to format and display the remaining time.
pauseAlarm() resets currentTime to 0 and updates the display.

If currentTime were declared as a local variable inside setAlarm(), it would be inaccessible to the interval callback (unless captured via closure, but then it couldn't be modified by pauseAlarm() or reset across multiple alarm sets).
Each call to setAlarm() would create a new, independent local variable, breaking the ability to stop or reset the timer from outside. By making it a module level variable, all relevant functions share a single source of true for the remaining time, allowing consistent updates and control.

[cjyuan]

Currently when starting a new countdown, the application does not always return to a clean initial state,
which can lead to inconsistent behaviour between runs.

Hint: a user may not click the "Stop" button first before starting a new count down.

Can also consider introducing a dedicated reset function to return the app to a clean initial state to help ensure consistency. There are few places in this script you can call this reset function instead of repeating the reset logic.

[carlosyabreu]

Thanks for pointing that out to me.

[cjyuan]

Have you figured out what else need to be reset before a new countdown begins?

[cjyuan]

What range of integers are acceptable? (Currently some input can make the app behave abnormally)

[carlosyabreu]

The acceptable range of integers for the alarm clock is 0 or any positive integer (i.e., totalSeconds >= 0).
Negative integers cause abnormal behavior because:
The updateTimeDisplay function will produce malformed strings like -1:-1 (since Math.floor(-1/60) = -1 and -1 % 60 = -1).
The countdown interval checks if (currentTime > 0), which is false for negative values, so the timer never counts down and never triggers the alarm.
Zero is acceptable: it displays 00:00 and does not play the alarm (as expected for an already‑expired timer). Non‑integer numbers are truncated by parseInt, so only the integer part matters.

[cjyuan]

Why not just treat negative input as if they are NaN or just return immediately?

[cjyuan]

If input is 0, the timer interval will remain active even though we won't notice anything visually.

[carlosyabreu]

Thanks for pointing that out it makes the understanding more clear to me.
Looking again to the code I don't think so, I think it is not correct to have the timer interval remain active when the input is 0.
When totalSeconds = 0, the interval starts running but the condition if (currentTime > 0) is never true, so the interval never clears itself. This results in a useless, permanent interval that consumes computer resources unnecessarily.
The expected behavior should be:
if the input is 0, the display should show 00:00 and no interval should be started (and no alarm should play).

[cjyuan]

I just noticed your check on line 27.

Different issue now. When input is 0. the alarm won't sound but the interval won't stop.
Visually the user may not notice anything but the active interval will silently consume (very tiny amount) of CPU.

[cjyuan]

To better separate presentation logic from application logic, you can consider defining a CSS class, and use classList.toggle() to apply/remove the style. For example,

document.body.classList.toggle("alarm-activated", true);  // apply style
document.body.classList.toggle("alarm-activated", false); // remove style

[carlosyabreu]

Thank you @cjyuan.
Your observation adds value to my understanding.

[cjyuan]

Delete line 84 and then move the declaration of currentTime from line 2 to inside setAlarm(), and your code will still work.

No change required. You can take time to find out why later. It will help your understanding of the JS language better.

[cjyuan]

Your app is working but the code has some room to improve.

I will mark this PR as complete first. You can address the comments later.