London | 26-ITP-Jan | Abdul Moiz | Sprint 2 | Sprint 2#911
London | 26-ITP-Jan | Abdul Moiz | Sprint 2 | Sprint 2#911A-Moiz wants to merge 8 commits intoCodeYourFuture:mainfrom
Conversation
A-Moiz
added
the
Needs Review
Sprint-2/1-key-errors/0.js
Outdated
|
|
||
| // =============> write your new code here | ||
| function capitalise(str) { | ||
| let newStr = `${str[0].toUpperCase()}${str.slice(1)}`; |
There was a problem hiding this comment.
For a variable not needed to be reassigned a value, a better practice is to use const instead of let.
| console.assert(formatAs12HourClock("01:00") === "01:00 am"); | ||
| console.assert(formatAs12HourClock("11:59") === "11:59 am"); | ||
|
|
||
| // Noon | ||
| console.assert(formatAs12HourClock("12:00") === "12:00 pm"); | ||
| console.assert(formatAs12HourClock("12:30") === "12:30 pm"); | ||
|
|
||
| // Afternoon / evening | ||
| console.assert(formatAs12HourClock("13:00") === "1:00 pm"); |
There was a problem hiding this comment.
Output on lines 56 and 48 have slightly different format.
Can you modify the function to return a string in a consistent format?
There was a problem hiding this comment.
Hi I'm unsure of the question you're asking. Line 56 tests if 13:00 is same as 1:00pm and line 48 tests if 01:00 is same as 01:00 am. All the assertions are passing currently.
There was a problem hiding this comment.
If "01:00" is converted to "01:00 am", it is reasonable for the caller to expect "13:00" to be converted to "01:00 pm".
What could go wrong? Here are two examples,
- When the strings are all centered within a table column on a webpage, "01:00 pm" and "1:00 pm" would not align as nicely.
- When the times are compared in the program,
"1:00 pm" < "11:00 pm"and01:00 pm" < "11:00 pm"produce different results.
Consistency is important so the caller can be certain what to expect from a function.
Did you choose the format "1:00 pm" by design (before you implement the function), or did you set the expected value in your tests because you knew that's what your function will return?
| // Updated function: | ||
| function formatAs12HourClock(time) { | ||
| const hours = Number(time.slice(0, 2)); | ||
| const minutes = time.slice(3, 5); |
There was a problem hiding this comment.
Can also consider time.slice(-2) (a bit more expressive).
cjyuan
added
Reviewed
A-Moiz
added
Needs Review
|
Changes look good. Stretch exercise is optional so I will mark this PR as complete. |
cjyuan
added
Complete
2 participants
Learners, PR Template
Self checklist
Changelist
Completed all the exercises in Sprint 2 directory
Questions
N/A