WM | ITP-JAN-25 | HATEF_EIDI | Module-Data-Groups | Sprint 3| Alarm Clock #405

HatefEidi · 2025-03-18T12:38:16Z

Learners, PR Template

Self checklist

[*] I have committed my files one by one, on purpose, and for a reason
[* ] I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
[ *] I have tested my changes
[ *] My changes follow the style guide
[ *] My changes meet the requirements of this task

Changelist

Alarm Clock project, where the user type the number of seconds in an input, time gets decremented by every second and when time remaining reaches to 0, alarm goes off, and they can stop the alarm by clicking the stop alarm button.

Questions

I couldn't use the jest file, I have no idea how test looks like for testing JavaScript DOM, do we come across it later?

illicitonion

This is generally looking good - I left some comments on a few things to think about.

On the testing - how are you trying to run the tests? What happens when you try? Do you get error messages?

illicitonion · 2025-04-02T17:29:45Z

Sprint-3/Prep/index.html

It looks like you included files from the Prep in this PR, which should only include the Alarm Clock exercise - please can you remove the prep files from this PR?

I tried to get rid of the Prep file, however since I pushed the commits to the parent branch (main) whenever I request a pull, the parent changes appear to the PR.

Oh actually I need the title to immediately updates when the set alarm button is clicked in order to pass all the test, without line 6, the code doesn't pass 2 tests, and without line 15 the title don't get updated, so we need both, therefore I changed it back to what it was.

I'd recommend sitting with someone in class on Saturday, or on a call, to try to get this fixed. You should be able to remove this file just on your branch. But also if you've accidentally pushed it to your main, you should be able to remove it from there too.

Sprint-3/alarmclock/alarmclock.js

illicitonion · 2025-04-02T17:34:18Z

Sprint-3/alarmclock/alarmclock.js

+    const diff = alarmTime - currentTime;
+
+    //Updating the time remaining text in the HTML element
+    document.getElementById('timeRemaining').innerText="Time Remaining: "+timeFormatter(diff);


This looks like the same code as is on line 6.

Let's imagine we wanted to change the text it showed from "Time Remaining:" to "Time Left:" - we would need to make sure we remembered to change both lines 6 and 18.

Can you think of a way to make it so that we would only need to change one line of code if we wanted to make that change, rather than two?

The reason is as soon as the user set alarm the value is added to the title, I can only keep line 18, but the title updates after 1 second interval.
Now removed line 6, which means the title is updated after 1 second, if that needs to be changed and you prefer the user to see the title as soon as they click set alarm button, please let me know so I changed it to what it was before.

I agree that you need to update the element both when the alarm is set, and when it changes. What I'm asking here is: How can you do that without repeating exactly the same code?

Sprint-3/alarmclock/alarmclock.js

illicitonion · 2025-04-02T17:36:42Z

Sprint-3/alarmclock/alarmclock.js

+function timeFormatter(seconds) {
+  const remainingSeconds= seconds % 60;
+  const remainingMinutes = Math.floor(seconds / 60);
+  return `${remainingMinutes.toString().padStart(2, '0')}:${remainingSeconds.toString().padStart(2, '0')}`;


This is a bit hard to read because there's quite a lot on one line inside one string literal - could you think of a way to split this up a bit so that there isn't so much going on in one line?

Sure, created a separate function which takes two parameter, min and sec, and return the formatted time to time formatter function

These are much easier to read - thanks!

illicitonion · 2025-04-03T16:43:20Z

Sprint-3/alarmclock/alarmclock.js

+    const diff = alarmTime - currentTime;
+
+    //Updating the time remaining text in the HTML element
+    document.getElementById('timeRemaining').innerText="Time Remaining: "+timeFormatter(diff);


I agree that you need to update the element both when the alarm is set, and when it changes. What I'm asking here is: How can you do that without repeating exactly the same code?

illicitonion · 2025-04-03T16:44:33Z

Sprint-3/alarmclock/alarmclock.js

+}
+
+//formatting remaining seconds in the format of MM:SS
+function timeFormatter(seconds) {


We normally name functions like verbs because they do things - so formatTime rather than timeFormatter. (Same comment applies to timePadder)

illicitonion · 2025-04-03T16:45:27Z

Sprint-3/Prep/index.html

I'd recommend sitting with someone in class on Saturday, or on a call, to try to get this fixed. You should be able to remove this file just on your branch. But also if you've accidentally pushed it to your main, you should be able to remove it from there too.

illicitonion · 2025-04-03T16:46:12Z

Sprint-3/alarmclock/alarmclock.js

+function timeFormatter(seconds) {
+  const remainingSeconds= seconds % 60;
+  const remainingMinutes = Math.floor(seconds / 60);
+  return `${remainingMinutes.toString().padStart(2, '0')}:${remainingSeconds.toString().padStart(2, '0')}`;


These are much easier to read - thanks!

HatefEidi added 2 commits March 14, 2025 19:33

Prep First attempt

82b56f5

Alarm Clock Project

65302ea

HatefEidi requested review from KudahShambare and MorganDavid March 18, 2025 12:38

HatefEidi added 🎯 Topic Problem-Solving Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. 📅 Sprint 3 Assigned during Sprint 3 of this module labels Mar 18, 2025

HatefEidi added 2 commits March 18, 2025 13:09

Better jest result, formatting the header

77aec9a

Without Date.now() function

c35e44e

illicitonion 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 2, 2025

illicitonion reviewed Apr 2, 2025

View reviewed changes

illicitonion 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 2, 2025

Pass all the tests

d9b788a

illicitonion reviewed Apr 3, 2025

View reviewed changes

illicitonion removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 3, 2025

illicitonion removed the 🎯 Topic Problem-Solving label Aug 14, 2025

illicitonion closed this Sep 1, 2025

Uh oh!

WM | ITP-JAN-25 | HATEF_EIDI | Module-Data-Groups | Sprint 3| Alarm Clock #405

WM | ITP-JAN-25 | HATEF_EIDI | Module-Data-Groups | Sprint 3| Alarm Clock #405

Uh oh!

Conversation

HatefEidi commented Mar 18, 2025

Learners, PR Template

Changelist

Questions

Uh oh!

illicitonion left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants