Glasgow | Jan-26 | Elisabeth Matulian | Sprint 2 | Coursework#959
Glasgow | Jan-26 | Elisabeth Matulian | Sprint 2 | Coursework#959Elisabeth-Matulian wants to merge 6 commits intoCodeYourFuture:mainfrom
Conversation
Elisabeth-Matulian
added
the
Needs Review
|
|
||
| function formatAs12HourClock(time) { | ||
| const hours = Number(time.slice(0, 2)); | ||
| const mins = time.slice(3, 5); |
There was a problem hiding this comment.
Could also use time.slice(-2) to extract two characters from the right (or last two characters).
| return `${hours - 12}:${mins} pm`; | ||
| } | ||
| else if (hours == 0) { | ||
| return `${hours + 12}:${mins} am`; |
There was a problem hiding this comment.
0 +12 is 12. Why not just write 12 directly?
There was a problem hiding this comment.
Indeed. I didn't even notice.
| if (hours > 12) { | ||
| return `${hours - 12}:00 pm`; | ||
| return `${hours - 12}:${mins} pm`; | ||
| } |
There was a problem hiding this comment.
If "01:00" is converted to "01:00 am", it is probably reasonable for the caller to expect "13:00" to be converted to "01:00 pm".
When the returned values are not formatted consistently, it may result in unintended side-effect. For examples,
- When the strings are displayed, "01:00 pm" and "1:00 pm" would not align as nicely.
01:00 am
1:00 pm
12:00 am
01:00 pm
- When the formatted strings are compared in the program,
"1:00 pm" < "11:00 pm"and01:00 am" < "11:00 am"produce different results.
Consistency is important so the caller can be certain what to expect from a function.
There was a problem hiding this comment.
Thanks for the feedback, I've followed all comments. Could you take another look when you have a moment?
cjyuan
added
Reviewed
Elisabeth-Matulian
added
the
Needs Review
|
Changes look good. Well done. |
cjyuan
added
Complete
2 participants
Self checklist
Changelist
Completed all exercises
Questions
No questions