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 8 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
8 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
68d0b4c
added and removed a few tests
zakiaao-tech Oct 9, 2026
525f9fc
ordered and structured my function
zakiaao-tech Oct 9, 2026
701e223
added more tests
zakiaao-tech Oct 26, 2026
373fee6
fixed line 27
zakiaao-tech Oct 26, 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
27 changes: 23 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,30 @@
function formatAs12HourClock(time) {

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

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

Note: The .slice() method supports negative indices, which count positions from the end of the string.

For example, str.slice(-3) returns the substring containing last three characters from str.


if (hours === 0) {
return `12:${minutes} am`;
}

if (hours === 12) {
return `12:${minutes} pm`;
}

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

@cjyuan cjyuan Oct 12, 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 is also the name of the parameter.

Could you explain why using the same name for a local variable and a parameter is considered bad practice?

@zakiaao-tech zakiaao-tech Oct 27, 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.

Time is already the parameter in my function, declaring it again in the same scope can cause naming conflict.


if (time < 10) {
return `0${time}:${minutes} pm`;
}

return `${hours - 12}:${minutes} pm`;
Comment on lines +16 to +20

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

Could you try simplifying the code on lines 16-20 into one line of code by taking advantage of a String's built-in method (function) ?

}
return `${time} am`;

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

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

export {formatAs12HourClock};
export { formatAs12HourClock };
38 changes: 33 additions & 5 deletions format-clock-edge-cases/timeConverter.test.js
View file
Open in desktop
Comment thread
cjyuan marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -1,11 +1,39 @@
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("converts 14:00 to 02:00 pm ", function () {
assert.equal(formatAs12HourClock("14:00"), "02: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 () {
assert.equal(formatAs12HourClock("12:01"), "12:01 pm");
});

test("converts 00:00 to 12:00 am ", function () {
assert.equal(formatAs12HourClock("00:00"), "12:00 am");
});

test("converts 11:59 to 11:59 am ", function () {
assert.equal(formatAs12HourClock("11:59"), "11:59 am");
});
Comment on lines +17 to +31

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

Duplicate test.

@zakiaao-tech zakiaao-tech Oct 26, 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.

Thank you for the feedback. I've made changes, added the requested tests, and run node --test to check result.


test("can convert 00:34 to 12:34 am ", function () {
assert.equal(formatAs12HourClock("00:34"), "12:34 am");
});

test("converts 12:00 to 12:00 pm ", function () {
assert.equal(formatAs12HourClock("12:00"), "12:00 pm");
});
Loading