London | 26-ITP-Sep | Alan Mak | Sprint 1 | formatAs12HourClock Coursework by AlanGit-debug2604 · Pull Request #1641 · CodeYourFuture/Module-Structuring-and-Testing-Data

AlanGit-debug2604 · 2026-10-04T08:14:16Z

Alan Mak, 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

Edit lines in timeConverter.js for edge cases within valid inputs. Edit lines in timeConverter.test.js for five additional test cases. All tests run successfully.

Questions

For the purpose of stretching this task, I am researching on time Converter logic other than using if and else if, which produce long chunk of code lines when condition expands. I am wondering the readability for reviewers using key word switch?

…in 12-hour clock conversion

…hour === 12 for noon

…due characters morning, noon time, and shows minutes

…2604/Module-Structuring-and-Testing-Data into Sprint1-Coursework

abdishakoor-dev

Your function now handles all three bugs from the starter: minutes are kept after noon, 12:xx is pm, and 00:xx is 12 am. Testing the hours with one digit and two digits separately (lines 17 and 21) was a good way to look for edge cases.

Three things before I can mark this Complete:

timeConverter.test.js line 10: the expected value of a starter test has changed. See my comment there.
.gitignore is changed in this PR, but it isn't part of the task. See my comment there.
Two edge cases have no test yet. See my comment on timeConverter.test.js line 22.

On your question about switch: a plain switch (hours) compares hours against each case with ===, so it suits exact values like 12 but not ranges like hours > 12. There is a switch (true) pattern that allows ranges, but for conditions like yours most people find if / else if easier to read. I think having fewer branches would help readability more than changing the keyword. My comment on timeConverter.js line 10 is a place to start.

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

abdishakoor-dev · 2026-10-05T08:10:10Z

-test("can correctly convert morning time", function() {
-    assert.equal(formatAs12HourClock("08:00"), "08:00 am");
+test("can correctly convert morning time", function () {
+  assert.equal(formatAs12HourClock("08:00"), "8:00 am");


The starter expected "08:00 am" here, with the zero. The starter tests describe what the function must return, so they stay as written. Please put back "08:00 am" and run the tests. It will fail, and that's fine: it means timeConverter.js is what needs to change. Then check that line 18 uses the same format.

Fixed two morning time test cases back to starter tests expected results e.g. "08:00am". See changes on line 10 and 18

Also ran an initial test after correction, validated two morning time branches need to change.

Fixed now. Good.

abdishakoor-dev · 2026-10-05T08:10:10Z

 .vscode
-**/.DS_Store
+**/.DS_Store
+Prep/


This PR should only change files in format-clock-edge-cases/. To undo this change, run git checkout main -- .gitignore on your branch, then commit and push. If you want git to ignore your Prep/ folder only on your own computer, you can add it to .git/info/exclude instead. That file is never committed.

Ran undo change, committed, and pushed.

Also added Prep to last line of .git/info/exclude

Gone from the PR now. Thanks.

abdishakoor-dev · 2026-10-05T08:10:10Z

+  } else if (hours === 12) {
+    return `12:${minutes} pm`;
+  } else if (hours > 9) {
+    return `${hours}:${minutes} am`;


Lines 10 and 14 return the same template. Is there any input that reaches line 10 and would give a different answer if it went to line 14 instead? If not, do you need this branch?

Subsequent to put back starter tests expecting results (0X:00 am/pm) on single hour scenarios, keep existing branches to distinct where hours >12 (for pm) and hours <=9 (for am).

Also added hours >21 branch to keep 10pm and beyond in place rather than showing 010:00pm. Validated and passed in the first test case in test.js

Agreed, the two branches give different answers now.

abdishakoor-dev · 2026-10-05T08:10:10Z

+});
+
+test("can correctly convert dual characters hour morning time", function () {
+  assert.equal(formatAs12HourClock("11:00"), "11:00 am");


Bugs often hide at the exact point where something changes. 11:00 is an hour before am turns into pm. What is the very last minute before that happens? And what is the last minute of the whole day, just before it goes back to am? Each of those is worth its own test.

Added two test cases for edge scenarios: last minutes in each session of the day.

Both tests passed.

Those are the two. Good.

abdishakoor-dev · 2026-10-05T08:10:10Z

+});
+
+test("can correctly shows minutes input", function () {
+  assert.equal(formatAs12HourClock("16:59"), "4:59 pm");


Optional, not needed for Complete: once morning times keep their zero ("08:00 am"), should 16:59 give "4:59 pm" or "04:59 pm"? Whichever you pick, it's worth being the same for am and pm.

Keep converter and test align to stay result time format the same as 0X:00 for am and pm.

Validated and passed in test file.

Consistent now. Good.

…than 21, to line up with expect time formatting in test file.

abdishakoor-dev

Everything from last round is fixed, and you now use the same zero-padded format for am and pm, so the output reads consistently across the day.

Two things left before I can mark it Complete:

timeConverter.test.js lines 29 and 33 are the same test. See my comment there.
A few boundaries still have no test: a time just after midnight, just after noon, and the first minute of the afternoon. See my comment on line 14. I missed these last time, sorry.

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

abdishakoor-dev · 2026-10-05T15:36:43Z

+  assert.equal(formatAs12HourClock("12:00"), "12:00 pm");
+});
+
+test("can correctly convert to single character afternoon time", function () {


Lines 29 and 33 both call formatAs12HourClock("16:59") and expect the same answer. What does the second one check that the first doesn't? Keep one, or change one to an input that tests something new.

Changed the hour in second test on line 33 - to test minutes input shows correctly.

Test passed.

13:30 tests something new. Good.

abdishakoor-dev · 2026-10-05T15:36:43Z

+});
+
+test("can correctly convert midnight time", function () {
+  assert.equal(formatAs12HourClock("00:00"), "12:00 am");


Your 00:00 and 12:00 tests both use 00 minutes. What happens with 00:30 or 12:01? Each goes through a branch that builds the minutes on its own. And 13:00 is the very first time that reaches your hours > 12 branch. Each of those is worth its own test.

Added additional test cases for last minute of morning, last minute of the day, first minute of the day, and first minute after noon.

All tests passed.

All three are there now. Good.

… for edge times

abdishakoor-dev

The three boundaries are tested now, and the duplicate is gone. Your tests cover every point where the clock changes, from 00:01 through to 23:59. Nothing left to do, marking this Complete. Well done.

AlanGit-debug2604 and others added 8 commits October 2, 2026 09:49

add Prep/ directory to .gitignore

ae73566

timeConverter.js fix edge cases for hours ,and put minutes for input …

62033c7

…in 12-hour clock conversion

fix return for hours input 01to09 as 0${hours}:${minutes} am;

bcf5419

timeConverter.js fixed single hour time pm formats; added edge cases …

8b179f7

…hour === 12 for noon

test.js added five extra tests namely midnight, single hour morning, …

f6b1388

…due characters morning, noon time, and shows minutes

Merge branch 'Sprint1-Coursework' of https://github.com/AlanGit-debug…

9bb46ff

…2604/Module-Structuring-and-Testing-Data into Sprint1-Coursework

timeConverter.js to simplify hour formatting logic for hours > 12

8255bd2

line ending convention change from CRLF to LF for both coursework files

fee5fba

AlanGit-debug2604 added 📅 Sprint 1 Assigned during Sprint 1 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 4, 2026

abdishakoor-dev reviewed Oct 5, 2026

View reviewed changes

abdishakoor-dev added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 5, 2026

AlanGit-debug2604 added 4 commits October 5, 2026 11:33

fix: standardize hour formatting for morning times in tests

dcf5d9f

timeConverter.js fix: adjust hour formatting logic for times greater …

b350b54

…than 21, to line up with expect time formatting in test file.

test: add cases for afternoon and last minute time conversions

c4cfbd3

fix: remove redundant entry for Prep/ from .gitignore

7eb6e65

AlanGit-debug2604 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 5, 2026

abdishakoor-dev reviewed Oct 5, 2026

View reviewed changes

abdishakoor-dev removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 5, 2026

test: correct afternoon time conversion and add additional test cases…

a08f8d8

… for edge times

AlanGit-debug2604 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 5, 2026

abdishakoor-dev reviewed Oct 6, 2026

View reviewed changes

Conversation

AlanGit-debug2604 commented Oct 4, 2026

Alan Mak, PR Template

Task code

Changelist

Questions

Uh oh!

abdishakoor-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abdishakoor-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abdishakoor-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants