Skip to content

London | 26-ITP-Sep | Hanna Bohlin | Sprint 1 | formatAs12HourClock #1628

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

London | 26-ITP-Sep | Hanna Bohlin | Sprint 1 | formatAs12HourClock #1628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
58741a8
Add test to convert noon time, "12:00" (which fails)
habohlin Sep 28, 2026
53b309b
Refactor the previous test funcs to use arrow funcs
habohlin Sep 28, 2026
6e1e485
Add else clause to function to properly format noon time
habohlin Sep 28, 2026
c0c27ea
Add test for time with minutes other than 00, (it fails)
habohlin Sep 28, 2026
99591de
Add possible preceding 0 and exact minutes to times after noon
habohlin Sep 28, 2026
891851b
Add test for morning time complex mins (it passes)
habohlin Sep 28, 2026
8c42ec0
Add test for early noon complex minutes (it passes)
habohlin Sep 28, 2026
72e8b26
Add test for time between midnight and 1 am (it fails)
habohlin Sep 28, 2026
ebfc759
Add else if for when hour = 0, now the final test passes
habohlin Sep 28, 2026
7a51317
Reformatted with prettier
habohlin Sep 28, 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
9 changes: 6 additions & 3 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,14 @@
function formatAs12HourClock(time) {

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

if (hours > 12) {

@illicitonion illicitonion Sep 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I notice that in several of your branches you're doing the same thing - writing time.slice(-2) - if you had to change that for some reason, you'd need to change both copies. Can you think how to avoid this duplication?

return `${hours - 12}:00 pm`;
return `${hours - 12 < 10 ? "0" : ""}${hours - 12}:${time.slice(-2)} pm`;
} else if (hours === 12) {
return `${time} pm`;
} else if (hours === 0) {
return `12:${time.slice(-2)} am`;

@illicitonion illicitonion Sep 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've noticed that all of these branches have something in common - at its core, all of them are calculating an "hours", calculating a "minutes", and appending an "am" or "pm"

Often it can be useful to make clear in code what things are the same and what things are different. Can you think how you may structure this code so that you always just return ${hours}:${minutes} am/pm, but make clear with your if statement how you're differently computing those things?

}
return `${time} am`;
}

export {formatAs12HourClock};
export { formatAs12HourClock };
27 changes: 20 additions & 7 deletions format-clock-edge-cases/timeConverter.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,24 @@
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", () =>

@illicitonion illicitonion Sep 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks like a pretty thorough set of tests - well done!

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 correctly convert morning time", () =>
assert.equal(formatAs12HourClock("08:00"), "08:00 am"));

test("can correctly convert noon time", () =>
assert.equal(formatAs12HourClock("12:00"), "12:00 pm"));

test("can format afternoon time with minutes other than 00", () =>
assert.equal(formatAs12HourClock("15:45"), "03:45 pm"));

test("can format morning time with complex minutes", () =>
assert.equal(formatAs12HourClock("08:25"), "08:25 am"));

test("can format early noon complex minutes", () =>
assert.equal(formatAs12HourClock("12:17"), "12:17 pm"));

test("can format between midnight and 1 am", () =>
Comment on lines +14 to +23

@illicitonion illicitonion Sep 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These are really good tests but you're using inconsistent terminology here - sometimes you're saying "minutes other than 00" and other times "complex minutes". By using different terms it makes me as a reader wonder whether they have different meanings. If you mean the same thing, I'd recommend using the same term.

assert.equal(formatAs12HourClock("00:15"), "12:15 am"));
Loading