Sheffield | 25-ITP-SEP | Declan Williams | Sprint 3 | Alarm clock #846
Conversation
…d decrementing the timer
…d counting down in decrements
…when reaching zero
…unction every 1000ms
CatchingKiyoko
added
Needs Review
There was a problem hiding this comment.
Reviewed
Overall, this is definitely a good work. The alarm functionality works well, and your code is readable with clear variable names and helpful comments.
I've left a couple of questions in the code about:
- A bug that occurs when setting multiple alarms
- How to improve readability with magic numbers
Take a look at those comments and let me know what you think. Try to reproduce the bug and see if you can figure out why it happens before checking the solutions.
| } else { | ||
| updatedTime(); | ||
| } | ||
| }, 1000); // updates the function by running it every 1000ms |
There was a problem hiding this comment.
Here, 1000 is what we call a magic number (a value whose meaning isn't immediately obvious). How could you make this number's purpose clearer to someone reading your code?
| @@ -1,4 +1,50 @@ | |||
| function setAlarm() {} | |||
| function setAlarm() { | |||
There was a problem hiding this comment.
There is a bug in your program, suppose the user clicks on the Set Alarm button while the previous alarm is running.
- Why does that happen?
- How would you resolve this?
bp7968h
added
Reviewed
3 participants
Learners, PR Template
Self checklist
Changelist
added and implemented lines of code and refactored
ensured that errors throw NaN for when its not a number.