Repository navigation
London | 26-ITP-Sep | Shirin Panahian | Sprint 1 | Exhaustively test and fix formatAs12HourClock - #1633
London | 26-ITP-Sep | Shirin Panahian | Sprint 1 | Exhaustively test and fix formatAs12HourClock#1633shirinpanahian wants to merge 6 commits into
Conversation
shirinpanahian
added
the
Needs Review
| if (hours === 0) { | ||
| return `${String(hours + 12).padStart(2, "0")}${time.slice(2, 5)} am`; | ||
| } | ||
| if (hours === 12) { | ||
| return `${time} pm`; | ||
| } | ||
| if (hours > 12) { | ||
| return `${hours - 12}:00 pm`; | ||
| return `${String(hours - 12).padStart(2, "0")}${time.slice(2, 5)} pm`; | ||
| } |
There was a problem hiding this comment.
-
On line 4: Is it necessary to compute the "hour" part of the constructed string?
-
Could consider storing the minute in a variable first to avoid repeated code.
-
Note: The .slice() method supports negative indices, which count positions from the end of the string.
For example,str.slice(-3)returns the substring containing last three characters fromstr.
There was a problem hiding this comment.
Line 5 is much simpler now, and the minutes variable removes the repeat. Good.
cjyuan
added
Reviewed
shirinpanahian
added
Needs Review
|
Could you also address this comment? #1633 (comment) |
cjyuan
added
Reviewed
abdishakoor-dev
left a comment
abdishakoor-dev
left a comment
There was a problem hiding this comment.
Both of the earlier comments are sorted: every hour group now has a single-digit and a double-digit minute test, and storing the minutes once made the midnight line much simpler. One optional naming point inline.
Marking this Complete, well done.
| assert.equal(formatAs12HourClock("00:00"), "12:00 am"); | ||
| }); | ||
|
|
||
| test("can correctly convert minutes", function () { |
There was a problem hiding this comment.
Optional: "can correctly convert minutes" doesn't say what makes 21:08 worth testing. It's the last pm hour that becomes a single digit (9), with a single-digit minute. A name like "can correctly convert the last single-digit pm hour" or "can correctly convert 9pm with single-digit minutes" would say that, the way your other test names do.
abdishakoor-dev
added
Complete
3 participants
Learners, PR Template
Self checklist
Task code
CYF-1197
Changelist
Fixed formatAs12HourClock to correctly convert 24-hour times to 12-hour format and Added tests for the different edge cases.