Birmingham | 25-ITP-Sep | Joy Opachavalit | Sprint 3 | coursework/sprint-3 #775

enjoy15 · 2025-10-15T22:28:52Z

Learners, PR Template

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

-Completed all excercises in Sprint-3

…t-3-implement-and-rewrite/implement/1-get-angle-type

cjyuan · 2025-10-25T09:37:52Z

Please hold on creating new PRs for Sprint-3 exercise. I am asking for permission to accept also Sprint-3 exercise submitted in one PR.

On separate note, why set the branch name to coursework/sprint-3-implement-and-rewrite/implement/1-get-angle-type?

enjoy15 · 2025-10-25T10:18:12Z

Please hold on creating new PRs for Sprint-3 exercise. I am asking for permission to accept also Sprint-3 exercise submitted in one PR.

On separate note, why set the branch name to coursework/sprint-3-implement-and-rewrite/implement/1-get-angle-type?

Thanks for the feedback! I initially named the branch that way because I thought a separate PR was required for each file. Later, I realized that wasn’t necessary, but I continued working on the same branch. I wasn’t sure how to rename the branch, so I decided to keep it as is.

cjyuan · 2025-10-25T10:48:53Z

We could rename a branch on GitHub.
However, if we have opened a PR from a branch, renaming the branch will close the corresponding PR.

cjyuan

Let's continue to work on this branch. Should you need to resubmit your work in separate PRs, you can just submit the improved code.

Regarding the PR description, the "Changelist" header is not formatted properly in Markdown syntax.

cjyuan · 2025-10-25T10:50:20Z

Sprint-3/1-implement-and-rewrite-tests/implement/1-get-angle-type.js

+    return "Acute angle";
+  }
   // Then keep going for the other cases, one at a time.
+  if (angle > 90 && angle < 180) {


Checking angle > 90 is optional because previous if-statements (together with the return statements) can guarantee angle is always more than 90.

Removed checking angle > 90

Sprint-3/1-implement-and-rewrite-tests/implement/2-is-proper-fraction.js

cjyuan · 2025-10-25T10:56:22Z

Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js

+  const numValue = parseInt(rank, 10);
+  if (numValue >= 2 && numValue <= 10) {


In JavaScript, strings that represent valid numeric literals in the language can be safely converted to equivalent numbers. For examples, "0x02", "2.1", or "0002".

Does your function return the value you expected from each of the following function calls?

getCardValue("0x02♠"); getCardValue("2.1♠"); getCardValue("0002♠");

With the current function
getCardValue("0x02♠") → undefined
parseInt("0x02", 10) stops at "x" and returns 0, which is outside the 2–10 range, so function returns undefined.

getCardValue("2.1♠") → 2
parseInt("2.1", 10) parses up to the dot and returns 2, which is in the 2–10 range, so function returns 2.

getCardValue("0002♠") → 2
parseInt("0002", 10) returns 2, so the function returns 2.

I changed the code so "0x02" to be interpreted as the JavaScript numeric literal 0x02 (i.e. 2), and reject non-integer ranks like "2.1", by changing the parsing to use Number() and check integerness.### ###

cjyuan · 2025-10-25T10:58:03Z

Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/2-is-proper-fraction.test.js

+test("should return false for a negative fraction", () => {
+  expect(isProperFraction(-2, 3)).toEqual(false);
+});


In math, -2/3 is considered a proper fraction.

change the test result to true for-2/3.

cjyuan · 2025-10-25T11:02:34Z

Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/3-get-card-value.test.js

+test("should return numeric value for number cards (2-10)", () => {
+  const fiveofHearts = getCardValue("5♥");
+  expect(fiveofHearts).toEqual(5);
+});     


Could consider testing more sample values in each category (to make the test more comprehensive), especially the boundary cases such as "2♥" and "10♥".

Added more tests, especially the boundary cases such as "2♥" and "10♥."

cjyuan · 2025-10-25T11:04:56Z

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

+test("should return '2nd' for 2", () => {
+  expect(getOrdinalNumber(2)).toEqual("2nd");
+});


To ensure thorough testing, we need broad scenario coverage. 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"); });

Tests are now grouped together.

enjoy15 · 2025-10-25T11:37:00Z

Regarding the PR description, the "Changelist" header is not formatted properly in Markdown syntax.

Changelist updated. Thank you.

cjyuan

Changes look good. I have few more suggestions.

cjyuan · 2025-10-25T21:34:33Z

Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js

+const numeric = Number(rank);            
+if (Number.isInteger(numeric) && numeric >= 2 && numeric <= 10) {
+  return numeric;
+}


In JavaScript, strings that represent valid numeric literals in the language can be safely converted to equivalent numbers. To see some examples, you can request AI to show you all forms of string values in JS, when converted to a number, equals to an integer 2.

Do you want these kinds of strings be recognized as valid ranks?

Hint; Validate the string value before converting its value.

Change the code to validate the string value before converting its value.

cjyuan · 2025-10-25T21:49:42Z

Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/2-is-proper-fraction.test.js

 test("should return false for a negative fraction", () => {
-  expect(isProperFraction(-2, 3)).toEqual(false);
+  expect(isProperFraction(-2, 3)).toEqual(true);
 });


The test description does not match what is being tested.

You could consider "function call with negative parameters" as one category and test some samples in this category,
or you could introduce two tests, "should return false for negative improper fraction", "should return true for negative proper fraction".

The comments and tests for negative fractions are now clarified and split into two categories: negative proper fractions (should return true) and negative improper fractions (should return false), with representative samples for each.

cjyuan · 2025-10-25T21:57:33Z

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

+// All other numbers: should append 'th'
+test("append 'th' to all other numbers", () => {
+  expect(getOrdinalNumber(4)).toEqual("4th");
+  expect(getOrdinalNumber(10)).toEqual("10th");
+  expect(getOrdinalNumber(14)).toEqual("14th");
  expect(getOrdinalNumber(0)).toEqual("0th");
 });


A more informative description could be "... to numbers ending in 0, 4-9."

Comment updated. Thank you.

enjoy15 added 17 commits October 13, 2025 21:02

get angle case2

97d02c7

case3 update

bf6c370

update case 4

1a47995

update case 5

3be3bfb

2-fraction/case2

a7b7c2f

2-is-proper-fraction update case

e110722

10 card case

716f047

10 card case

138843b

Ace card

4879aaa

undefined case

d34d343

1 rewrite test with jest

4d61885

2-is proper-fraction test with jest

989855e

3-get card value test with jest

56625ec

2-practice-tdd/count test

47e385f

2-practice-tdd/repeat test

45dc764

2-practice-tdd/get ordinal number test

00e6c6d

Merge branch 'coursework/sprint-3-practice-tdd' into coursework/sprin…

35b6637

…t-3-implement-and-rewrite/implement/1-get-angle-type

enjoy15 mentioned this pull request Oct 15, 2025

25-ITP-Sept | Structuring and testing data | Joy Opachavalit enjoy15/My-Coursework-Planner#64

Open

enjoy15 added 📅 Sprint 3 Assigned during Sprint 3 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 15, 2025

cjyuan reviewed Oct 25, 2025

View reviewed changes

enjoy15 added 6 commits October 25, 2025 20:11

Remove checking for angle > 90

eb1d90f

convert negative numbers to positive before comparing

2f42a85

Change test result to true for -2/3

6db1bca

Change function getCardValue to use Number instead

8f61c0f

Add more tests

0e834a6

Group tests together

f14ec50

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

cjyuan reviewed Oct 25, 2025

View reviewed changes

cjyuan removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 25, 2025

enjoy15 added 3 commits October 26, 2025 04:07

Update comment ".. to numbers ending in 0, 4-9."

24bc0d7

Amend Function call with negative parameters

338eadc

validate string value before converting its value

36e1138

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

		const numValue = parseInt(rank, 10);
		if (numValue >= 2 && numValue <= 10) {

Uh oh!

Birmingham | 25-ITP-Sep | Joy Opachavalit | Sprint 3 | coursework/sprint-3 #775

Are you sure you want to change the base?

Birmingham | 25-ITP-Sep | Joy Opachavalit | Sprint 3 | coursework/sprint-3 #775

Conversation

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

Uh oh!

Learners, PR Template

Changelist

Uh oh!

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

Uh oh!

Uh oh!

enjoy15 commented Oct 25, 2025

Uh oh!

cjyuan commented Oct 25, 2025

Uh oh!

cjyuan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

enjoy15 commented Oct 25, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

enjoy15 commented Oct 15, 2025 •

edited

Loading

cjyuan commented Oct 25, 2025 •

edited

Loading

cjyuan left a comment •

edited

Loading