London | 26-ITP-Sept | Fung Nin Lee | Sprint 1| formatAs12HourClock

[leerogerfn]

Learners, PR Template

Self checklist

• I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
• My changes meet the requirements of the task
• I have tested my changes
• My changes follow the style guide

Task code

CYF-1197

Changelist

fixed the bugs for the formatAs12HourClock. Tested all edge-cases.

Questions

.

[abdishakoor-dev]

Your fix works for all three bugs in the starter. Minutes are kept after noon, 12:xx is now pm, and 00:xx is now 12 am.

Three things before I can mark this Complete:

1. The two tests from the starter are gone. See my comment on timeConverter.test.js line 5.
2. Some edge cases have no test yet. See my comment on line 21.
3. Both files fail Prettier. Prettier is a tool that formats your code to one agreed style. Then a reviewer only sees the changes you meant to make. Open each file you changed, right click, and choose Format Document. Pick Prettier if VS Code asks. To format every time you save, follow the steps here: https://github.com/CodeYourFuture/Module-Structuring-and-Testing-Data#2-enable-formatting-on-save

Add the Needs Review label again once you've pushed.

[abdishakoor-dev]

The starter file had two tests here:

• "23:00" should give "11:00 pm"
• "08:00" should give "08:00 am"

These tests show the format the function must give. There is a space before am and pm, and 08 keeps its zero.

Your code now gives "11:00pm" and "8:00am". So your tests check a different format.

Please put the two starter tests back. Then run the tests. Do they pass? If not, change the function, not the tests.

[abdishakoor-dev]

The two tests are back, but the expected values are not the starter's:

• Line 6 expects "11:00pm". The starter expects "11:00 pm".
• Line 10 expects "8:00am". The starter expects "08:00 am".

The tests describe what the function must return, so they stay as the starter wrote them. Please put back those exact two values, run the tests, and change timeConverter.js until they pass. Then check your other tests use the same format.

[abdishakoor-dev]

Line 6 looks good. Line 10 should still be "08:00 am" like the starter, with the zero.

Your test will fail once you change it back, and that's fine. It means the function is what needs fixing. Line 14 uses hours, which is a number, so 08 turns into 8. Have a look at how the starter returned morning times. It used time instead.

[abdishakoor-dev]

You test 12:12 and 00:12. Bugs often hide at the exact point where something changes. Here, that is where am changes to pm, and back again.

Which times are right at that point? For example, what about 11:59, 12:00, 23:59 and 00:00? Can you add a test for each one?

[abdishakoor-dev]

Good, these are the right times. One typo: line 18 passes "23:159", not "23:59". The test still passes, because your function only reads the last two characters for the minutes. Please fix the input.

[abdishakoor-dev]

Thanks, that's sorted.

[abdishakoor-dev]

Not needed for Complete, just a question. On a 24-hour clock, is 24:12 a real time? What comes one minute after 23:59?

The README says you do not need to handle invalid inputs. So you can also remove the invalid-input code and tests if you want.

[abdishakoor-dev]

Thanks. Both files pass Prettier now, and 11:59, 12:00, 23:59 and 00:00 are exactly the right edge cases to test.

Two things before I can mark this Complete:

1. timeConverter.test.js lines 6 and 10: the starter tests are back, but their expected values changed. See my reply on line 5.
2. timeConverter.test.js line 18: the input has a typo. See my reply on line 21.

Add the Needs Review label again once you've pushed.

[abdishakoor-dev]

Nearly there, the typo on line 18 is sorted. Just line 10 left, see my reply on line 5. Once that's done I'll mark this Complete.