Skip to content

Cape Town | 26-ITP-Sept | Leigh Ross | Sprint 1 | Exhaustively test and fix formatAs12HourClock #1646

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
leigh-ross wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Cape Town | 26-ITP-Sept | Leigh Ross | Sprint 1 | Exhaustively test and fix formatAs12HourClock #1646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b98a9fb
Fixed bugs and added more test cases
leigh-ross Oct 4, 2026
88f7443
Added more test cases with help from AI
leigh-ross Oct 6, 2026
e033237
NaN error fixed
leigh-ross Oct 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions format-clock-edge-cases/timeConverter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,25 @@
function formatAs12HourClock(time) {

const hours = Number(time.slice(0, 2));
const minutes = time.slice(-2);
let hours12;
let timePeriod;

if (hours > 12) {
return `${hours - 12}:00 pm`;
if (hours === 0) {
hours12 = "12";
timePeriod = "am";
}
else if (hours === 12) {
hours12 = "12";
timePeriod = "pm";
}
else if (hours < 12) {
hours12 = String(hours).padStart(2, "0");
timePeriod = "am";
}
return `${time} am`;
else if (hours > 12) {
hours12 = String(hours - 12).padStart(2, "0");
timePeriod = "pm"
}
Comment on lines +7 to +22

@cjyuan cjyuan Oct 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code works.

Could you use AI to explore alternatives to simplify the code on lines 4-22 into 3-4 lines of code?

@leigh-ross leigh-ross Oct 6, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what AI gave me:
Option 1: Ternary + modulo (3 lines)
const hours12 = hours % 12 || 12;
const timePeriod = hours < 12 ? "am" : "pm";
return ${String(hours12).padStart(2, "0")}:${minutes} ${timePeriod};

  • hours % 12 gives 0 for midnight/noon, so || 12 converts that to 12
  • hours < 12 correctly identifies am (0-11); everything else is pm

Option 2: Single expression (1 line)
return ${String(hours % 12 || 12).padStart(2, "0")}:${minutes} ${hours < 12 ? "am" : "pm"};

Option 3: With toLocaleTimeString (fully different approach)
const [h, m] = time.split(":").map(Number);
return new Date(2000, 0, 1, h, m).toLocaleTimeString("en-US", {
hour: "2-digit", minute: "2-digit", hour12: true
}).toLowerCase();

Out of those 3 options, I like option 1 the best:

  • It is more concise that my current code but there is still enough info from the variables for me to understand at a glance.
  • The hours 12 handling is also much shorter without using if - else blocks
  • I would still prefer to declare a function just to ensure that my program is not full of global variables.

Would you like me to change my code so that option 1 is implemented correctly?

@cjyuan cjyuan Oct 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code submitted to a PR should be one's best work.
So I think it is a good practice to apply what you think is an improved solution.

Also, by implementing them, you could strengthen you understanding.

I agree option 1 is better than then other two.

I would still prefer to declare a function just to ensure that my program is not full of global variables.

What do you mean? Why would the change involve any global variables?

return `${hours12}:${minutes} ${timePeriod}`
}

export {formatAs12HourClock};
export {formatAs12HourClock};
92 changes: 88 additions & 4 deletions format-clock-edge-cases/timeConverter.test.js
View file
Open in desktop

@cjyuan cjyuan Oct 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of the challenge in this exercise is to design a comprehensive test suite that can detect potential errors not only in your own function implementation but also in future modifications made by others.

What additional test data should be included to make the test suite more comprehensive?

@leigh-ross leigh-ross Oct 6, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added more test cases to really catch any further bugs. It was a bit difficult so, I used a bit of help from AI especially for that last 24 hour sweep test.
I expanded my tests with zero-padding tests for inputs such as "08:30" and "10:05".
I also added boundary tests around "noon" and "midnight".
The 24 hour sweep tests should cover everything else and ensure that any bugs will be caught.

@cjyuan cjyuan Oct 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a thorough test.

Did you pick up any thought process or logic from the AI for deriving the test data? This way, you'd have a better idea of what to test or check when implementing similar functions in the future.

Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,94 @@ import {formatAs12HourClock} from "./timeConverter.js";
import assert from "node:assert";
import test from "node:test";

test("correctly convert time after 12:00", function(){
assert.equal(formatAs12HourClock("23:00"), "11:00 pm");
// Edge cases ----------------------------------------------------------------------------------------------------------------------------
test("can correctly convert midnight", function () {
assert.equal(formatAs12HourClock("00:00"), "12:00 am");
});

test("can correctly convert morning time", function() {
assert.equal(formatAs12HourClock("08:00"), "08:00 am");
test("can correctly convert noon", function () {
assert.equal(formatAs12HourClock("12:00"), "12:00 pm");
});

// Zero-padding tests ----------------------------------------------------------------------------------------------------------------------------
test("pads single digit hours with a leading zero", function () {
assert.equal(formatAs12HourClock("01:00"), "01:00 am");
assert.equal(formatAs12HourClock("09:30"), "09:30 am");
});

test("pads single digit minutes with a leading zero", function () {
assert.equal(formatAs12HourClock("08:05"), "08:05 am");
assert.equal(formatAs12HourClock("00:05"), "12:05 am");
});

// Noon boundary tests ----------------------------------------------------------------------------------------------------------------------------
test("handles the minute just before noon", function () {
assert.equal(formatAs12HourClock("11:59"), "11:59 am");
});

test("handles the minute just after noon", function () {
assert.equal(formatAs12HourClock("12:01"), "12:01 pm");
});

// Midnight boundary tests ----------------------------------------------------------------------------------------------------------------------------
test("handles the minute just before midnight", function () {
assert.equal(formatAs12HourClock("23:59"), "11:59 pm");
});

test("handles the minute just after midnight", function () {
assert.equal(formatAs12HourClock("00:01"), "12:01 am");
});

// First pm hour after noon ----------------------------------------------------------------------------------------------------------------------------
test("converts 13:00 to 01:00 pm", function () {
assert.equal(formatAs12HourClock("13:00"), "01:00 pm");
});

// 24 hour sweep tests ----------------------------------------------------------------------------------------------------------------------------------
test("correctly converts every hour of the day", function () {
const expected = [
"12:00 am",
"01:00 am",
"02:00 am",
"03:00 am",
"04:00 am",
"05:00 am",
"06:00 am",
"07:00 am",
"08:00 am",
"09:00 am",
"10:00 am",
"11:00 am",
"12:00 pm",
"01:00 pm",
"02:00 pm",
"03:00 pm",
"04:00 pm",
"05:00 pm",
"06:00 pm",
"07:00 pm",
"08:00 pm",
"09:00 pm",
"10:00 pm",
"11:00 pm",
];

for (let hour = 0; hour < 24; hour++) {
const input = String(hour).padStart(2, "0") + ":00";
assert.equal(
formatAs12HourClock(input),
expected[hour],
`failed for input ${input}`
);
}
});

// Reason behind my test cases
// - Covers midnight and noon edge cases
// - Converts am time correctly
// - Converts pm time correctly
// - Converts time with minutes correctly
// - Verifies zero-padding on both hours and minutes
// - Verifies the exact am/pm boundary (11:59 -> 12:00 -> 12:01)
// - Verifies the exact midnight boundary (23:59 -> 00:00 -> 00:01)
// - Full 24-hour sweep to guard against future regressions
Loading