Birmingham | 25-ITP-Sept | Khor Biel | Sprint 3 | coursework/sprint-3-practice-tdd #801

wankoak · 2025-10-24T10:59:01Z

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

Changelist

Added and pass tests for countChar, getOrdinalNumber, and repeat functions

Questions

No questions

…tions

cjyuan

Why not include a brief description of the changes made in this PR in the PR description?

cjyuan · 2025-10-28T23:11:22Z

Sprint-3/2-practice-tdd/count.js

+    typeof stringOfCharacters !== "string" ||
+    typeof findCharacter !== "string"
+  ) {
+    return 0;


How should the caller distinguish the result of countChar(1234, 5) from the result of countChar("1234", "5")?

cjyuan · 2025-10-28T23:13:15Z

Sprint-3/2-practice-tdd/get-ordinal-number.js

+  if (typeof num !== "number" || isNaN(num)) {
+    return "";
+  }


3.1416 and Infinity could pass this test.

cjyuan · 2025-10-28T23:15:24Z

Sprint-3/2-practice-tdd/get-ordinal-number.test.js

+// Case 2: Identify the ordinal number for 2
+test("should return '2nd' for 2", () => {
+  expect(getOrdinalNumber(2)).toEqual("2nd");
+});
+


To ensure thorough testing, we need broad scenarios that cover all possible cases.
Listing individual values, however, can quickly lead to an unmanageable number of test cases.
Instead of writing tests for individual numbers, consider grouping all possible input values into meaningful categories.
Then, select representative samples from each category to test. This approach improves coverage and makes our tests easier to maintain.

For example, we can prepare a test for numbers 2, 22, 132, etc. as

test("append 'nd' to numbers ending in 2, except those ending in 12", () => { expect( getOrdinalNumber(2) ).toEqual("2nd"); expect( getOrdinalNumber(22) ).toEqual("22nd"); expect( getOrdinalNumber(132) ).toEqual("132nd"); });

cjyuan · 2025-10-28T23:15:45Z

Sprint-3/2-practice-tdd/get-ordinal-number.test.js

+test("should return 'th' for numbers ending with 0, 4–9", () => {
+  expect(getOrdinalNumber(10)).toEqual("10th");
+  expect(getOrdinalNumber(14)).toEqual("14th");
+  expect(getOrdinalNumber(19)).toEqual("19th");
+});


This test is good.

cjyuan · 2025-10-28T23:17:07Z

Sprint-3/2-practice-tdd/get-ordinal-number.test.js

+test("should return '11th', '12th', '13th' for special cases", () => {
+  expect(getOrdinalNumber(11)).toEqual("11th");
+  expect(getOrdinalNumber(12)).toEqual("12th");
+  expect(getOrdinalNumber(13)).toEqual("13th");
+});


When the person implementing the function see this test message, they may ask "what exactly are 'special cases'?".

github-actions · 2025-10-29T08:54:13Z

Your PR description contained template fields which weren't filled in.

Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

github-actions · 2025-10-29T10:23:41Z

Your PR description contained template fields which weren't filled in.

Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

github-actions · 2025-10-29T10:41:37Z

Your PR description contained template fields which weren't filled in.

Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

github-actions · 2025-10-29T10:41:41Z

Your PR description contained template fields which weren't filled in.

Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

cjyuan

Please update the PR description so that it can pass to automated check.

What does your "thumbs up" emoji mean? You reacted to my comment but did not make any change accordingly.

cjyuan · 2025-10-29T12:33:29Z

Sprint-3/2-practice-tdd/count.js

-  // Ensure valid inputs
-  if (
-    typeof stringOfCharacters !== "string" ||
-    typeof findCharacter !== "string"
-  ) {
-    return 0;
-  }
-


Why remove the check instead of strengthening it?

cjyuan · 2025-10-29T12:34:34Z

Sprint-3/2-practice-tdd/count.js

-  let count = 0;
-  for (let char of stringOfCharacters) {
-    if (char === findCharacter) {
-      count++;
+let total = 0;
+   for (let i=0; i < stringOfCharacters.length; i++){
+    if (findCharacter == stringOfCharacters[i]){
+      total++;
    }
-  }
-


Why replace the for-of loop with a for loop? The former is more compact.

cjyuan · 2025-10-29T12:35:08Z

Sprint-3/2-practice-tdd/get-ordinal-number.js

-  // Ensure the input is a valid number
-  if (typeof num !== "number" || isNaN(num)) {
-    return "";
-  }
-


Why remove the check instead of strengthening it?

@cjyuan I was just trying to simplify the code

cjyuan · 2025-10-29T12:39:09Z

Sprint-3/2-practice-tdd/get-ordinal-number.js

-  const lastTwoDigits = num % 100;
-  const lastDigit = num % 10;
+
+let result;

-  // Handle special cases: 11th, 12th, 13th
-  if (lastTwoDigits >= 11 && lastTwoDigits <= 13) {
-    return `${num}th`;
+  if (num % 100 == 11 || num % 100 == 12 || num % 100 == 13){
+    result = num.toString() + "th";


Why replace the previous approach

const lastTwoDigits = num % 100; if (lastTwoDigits >= 11 && lastTwoDigits <= 13) { return `${num}th`; ...

with the current approach

let result; if (num % 100 == 11 || num % 100 == 12 || num % 100 == 13){ result = num.toString() + "th"; ... return result;

I was just trying to simplify the code

github-actions · 2025-10-29T12:46:44Z

Your PR couldn't be matched to an assignment in this module.

Please check its title is in the correct format, and that you only have one PR per assignment.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

github-actions · 2025-10-29T12:46:48Z

Your PR description contained template fields which weren't filled in.

Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

wankoak · 2025-10-29T12:52:00Z

@cjyuan, just give me a few minutes. I'm in the market, I'll edit my code as soon as I get home.
The thumbs up means I understood what you said, and I will implement it.
And for the PR description it is very annoying how it keeps rejecting despite the fact that I removed the (Briefly explain your PR)
I'm trying @cjyuan just let me get home and fix the errors because I'm not with my laptop now

cjyuan · 2025-10-30T16:27:45Z

The functions work.

Some of the code I commented remain unchanged, so I am not sure if you have addressed all my comments. Can you address all my previous comments?

Added and pass tests for countChar, getOrdinalNumber, and repeat func…

effcc4b

…tions

wankoak added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 24, 2025

cjyuan reviewed Oct 28, 2025

View reviewed changes

cjyuan 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 28, 2025

Fixed some errors

be7308e

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

cjyuan reviewed Oct 29, 2025

View reviewed changes

cjyuan 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 29, 2025

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

cjyuan 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 30, 2025

Uh oh!

Birmingham | 25-ITP-Sept | Khor Biel | Sprint 3 | coursework/sprint-3-practice-tdd #801

Are you sure you want to change the base?

Birmingham | 25-ITP-Sept | Khor Biel | Sprint 3 | coursework/sprint-3-practice-tdd #801

Uh oh!

Conversation

wankoak commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelist

Questions

Uh oh!

cjyuan 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!

github-actions bot commented Oct 29, 2025

Uh oh!

github-actions bot commented Oct 29, 2025

Uh oh!

github-actions bot commented Oct 29, 2025

Uh oh!

github-actions bot commented Oct 29, 2025

Uh oh!

cjyuan 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!

github-actions bot commented Oct 29, 2025

Uh oh!

github-actions bot commented Oct 29, 2025

Uh oh!

wankoak commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cjyuan commented Oct 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wankoak commented Oct 24, 2025 •

edited

Loading

wankoak commented Oct 29, 2025 •

edited

Loading