Skip to content

London | ITP-JAN-2026 | Said Fayaz Sadat | Sprint 3 | coursework/AlarmClock#1084

Closed
fayaz551 wants to merge 5 commits intoCodeYourFuture:mainfrom
fayaz551:coursework/Sprint-3/Alarmclock
Closed

London | ITP-JAN-2026 | Said Fayaz Sadat | Sprint 3 | coursework/AlarmClock#1084
fayaz551 wants to merge 5 commits intoCodeYourFuture:mainfrom
fayaz551:coursework/Sprint-3/Alarmclock

Conversation

@fayaz551
Copy link
Copy Markdown

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

implemented the AlarmClock function to work properly with jest test cases.

Questions

n/a

@fayaz551 fayaz551 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 23, 2026
@Luro91 Luro91 added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Apr 4, 2026
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The html title should be "Alarm clock app"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated to "Alarm clock app"

@@ -1,4 +1,31 @@
function setAlarm() {}
function setAlarm() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when the user sets an alarm when the countdown is already running?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a user sets a new alarm while one is already running, the original code would create multiple overlapping timers.
I moved the timerInterval variable to a global scope and added clearInterval(timerInterval) at the start of the setAlarm function to fix it.

Comment thread Sprint-3/alarmclock/alarmclock.js Outdated
Comment on lines +17 to +20
const minutes = Math.floor(remainingTime / 60);
const seconds = remainingTime % 60;

heading.textContent = `Time Remaining: ${String(minutes).padStart(2, "0")}:${String(seconds).padStart(2, "0")}`;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This 3 lines of code are also repeated in line 24. How could you make the code reusable?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make the code reusable, I extracted the time formatting and DOM update logic into a separate helper function called updateDisplay(time).

const time = document.getElementById("alarmSet").value;
const heading = document.getElementById("timeRemaining");

let remainingTime = Number(time);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when the user enters a negative number or a string value?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a user enters a string, Number(time) results in NaN, causing the display to show 'NaN:NaN'. If they enter a negative number, the remainingTime <= 0 check passes immediately, triggering the alarm instantly without a countdown.

To prevent this, I added a validation check at the beginning of the function to ensure the input is a positive number before starting the interval.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done. It's important to validate user input. How could you inform the user about the incorrect data?

@Luro91 Luro91 added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Apr 4, 2026
@fayaz551 fayaz551 added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Apr 7, 2026
@Luro91 Luro91 added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Apr 8, 2026
@illicitonion
Copy link
Copy Markdown
Member

Closing PR because the January ITP run has finished. Feel free to re-open if you're still working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants