Skip to content

London | 26-ITP-Sep | Sakiya Mayow | Sprint 1 | Structuring and Testing Data #1639

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
zakiaao-tech wants to merge 4 commits into
base: main
Choose a base branch
from
Open

London | 26-ITP-Sep | Sakiya Mayow | Sprint 1 | Structuring and Testing Data #1639

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b80ce87
if statement added
zakiaao-tech Oct 3, 2026
4092ead
edge cases
zakiaao-tech Oct 3, 2026
ce94896
added boundary cases
zakiaao-tech Oct 5, 2026
379bec4
added minutes to function
zakiaao-tech Oct 5, 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
24 changes: 20 additions & 4 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,27 @@
function formatAs12HourClock(time) {

const hours = Number(time.slice(0, 2));
const minutes = time.slice(3, 5);

if (hours > 12) {
return `${hours - 12}:00 pm`;
const time = hours - 12;

if (time < 10) {
return `0${time}:${minutes} pm`;
}
return `${hours - 12}:${minutes} pm`;
}
if (hours === 0) {
return `12:${minutes} am`;
}
if (hours === 12) {
return `12:${minutes} pm`;
}
return `${time} am`;

if (hours < 10) {
return `0${hours}:${minutes} am`;
}

return `${time}: ${minutes} am`;

@abdishakoor-dev abdishakoor-dev 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.

time holds the whole input, for example "10:30". So ${time} already includes the minutes, and : ${minutes} then adds them a second time, with a space in front. That's how you get 10:30: 30 am.

For 10:00 to 11:59 the hours already have two digits, so nothing needs changing about them. What does ${time} on its own give you, and is that all you need before am?

}

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

@cjyuan cjyuan Oct 4, 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.

It seem your tests only cover two cases:

  • can correctly convert morning time
  • correctly convert time after 12:00

What about the boundary cases?

Could the function correctly handle the minute in the time? For examples, 00:34, 23:59?

@abdishakoor-dev abdishakoor-dev 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.

23:59 is tested now. 00:34 isn't yet; it's in the list in step 1 of my overall comment.

Original file line number Diff line number Diff line change
@@ -1,11 +1,35 @@
import {formatAs12HourClock} from "./timeConverter.js";
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");
test("correctly convert time after 12:00", function () {
assert.equal(formatAs12HourClock("23:00"), "11:00 pm");
});

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

test("correctly convert time after 12:00 ", function () {

@abdishakoor-dev abdishakoor-dev 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.

These four tests all have the same name, "correctly convert time after 12:00". If one of them failed, the output wouldn't tell you which time broke. They also all check a whole hour between 2pm and 9pm, so they all test the same thing.

Keep one of them, and give it a name that says the time it checks, for example "converts 14:00 to 02:00 pm". Delete the other three. Your new tests from step 1 of my overall comment test far more.

assert.equal(formatAs12HourClock("14:00"), "02:00 pm");
});

test("correctly convert time after 12:00 ", function () {
assert.equal(formatAs12HourClock("21:00"), "09:00 pm");
});

test("correctly convert time after 12:00 ", function () {
assert.equal(formatAs12HourClock("16:00"), "04:00 pm");
});

test("correctly convert time after 12:00 ", function () {
assert.equal(formatAs12HourClock("19:00"), "07:00 pm");
});

test("converts 23:59 to 11:59 ", function () {
assert.equal(formatAs12HourClock("23:59"), "11:59 pm");
});

test("converts 12:01 to 12:01 ", function () {

@abdishakoor-dev abdishakoor-dev 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.

Bugs often hide at the exact point where something changes. 12:01 is the first minute after noon, so the last minute before noon, 11:59, is just as important. Your function gets 11:59 wrong at the moment, and a test for it would have shown you. Midnight (00:00) and noon (12:00) are the other two points where the clock changes, so they need a test each too.

assert.equal(formatAs12HourClock("12:01"), "12:01 pm");
});
Loading