Glasgow | 26-ITP-May | Niangh Ciang | Sprint 2 | Coursework#1367
Glasgow | 26-ITP-May | Niangh Ciang | Sprint 2 | Coursework#1367Niangh-Ciang wants to merge 13 commits into
Conversation
Niangh-Ciang
added
📅 Sprint 2
| function formatAs12HourClock(time) { | ||
| const hours = Number(time.slice(0, 2)); | ||
| if (hours > 12) { | ||
| return `${hours - 12}:00 pm`; | ||
| } | ||
| if (hours === 12) { | ||
| return `${hours}:00 pm`; | ||
| } | ||
| if (hours === 0) { | ||
| const minutes = time.slice(3, 5); | ||
| return `12:${minutes} am`; | ||
| } | ||
| return `${time} am`; | ||
| } |
There was a problem hiding this comment.
First, without running the code, predict the output from the following function calls.
Then, check if your function is returning the value you expected?
console.log( formatAs12HourClock("12:35") );
console.log( formatAs12HourClock("01:01") );
console.log( formatAs12HourClock("13:01") );
There was a problem hiding this comment.
When I tested "12:35" and "13:01", the minutes were wrong because I originally put const minutes = time.slice(3, 5) inside the hours === 0 block, so only the midnight case kept the minutes.
For the other cases (hours > 12 and hours === 12), the function always returned :00.
I fixed it by moving const minutes to the top of the function and using ${minutes} in every return statement. Now all the tests pass.
cjyuan
added
Reviewed
Niangh-Ciang
added
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Changes look good. Your responses were clear and to the point. Excellent job!
|
|
||
| function formatAs12HourClock(time) { | ||
| const hours = Number(time.slice(0, 2)); | ||
| const minutes = time.slice(3, 5); |
There was a problem hiding this comment.
.slice() supports negative index. So we could also extract the last two characters as time.slice(-2).
| const currentOutput7 = formatAs12HourClock("01:01"); | ||
| const targetOutput7 = "01:01 am"; | ||
| console.assert( | ||
| currentOutput7 === targetOutput7, | ||
| `current output: ${currentOutput7}, target output: ${targetOutput7}` | ||
| ); | ||
| const currentOutput8 = formatAs12HourClock("13:01"); | ||
| const targetOutput8 = "1:01 pm"; | ||
| console.assert( | ||
| currentOutput8 === targetOutput8, | ||
| `current output: ${currentOutput8}, target output: ${targetOutput8}` | ||
| ); |
There was a problem hiding this comment.
If "01:01" is converted to "01:01 am", it is probably reasonable for the caller to expect "13:01" to be converted to "01:01 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.
cjyuan
added
Complete
2 participants
Self checklist
Changelist
Completed the courwork exercises in Sprint 2.