London | 26-ITP-Jan | Kris Oldrini | Sprint 3 | Implement And Rewrite Tests by XiaoQuark · Pull Request #1045 · CodeYourFuture/Module-Structuring-and-Testing-Data

XiaoQuark · 2026-02-24T17:26:46Z

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

1 Implement solutions

Implemented required functions for files:
- 1-get-angle-type.js
- 2-is-proper-fraction.js
- 3-get-card-value.js
Added tests in the above files to cover all possible cases (including error handling).
Verified all tests passed.

2 Rewrite tests with Jest

Rewrote equivalent test suites in Jest
Verified all tests pass via npm test

… about cases with numerator = 0

netEmmanuel · 2026-03-05T19:13:04Z

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

 // TODO: Write tests in Jest syntax to cover all combinations of positives, negatives, zeros, and other categories.

 // Special case: numerator is zero
-test(`should return false when denominator is zero`, () => {


Is there a reason why this test was deleted?

XiaoQuark · 2026-03-05T23:44:28Z

I'm not sure I understand this github message. Is there something I need to do?

netEmmanuel · 2026-03-06T16:06:25Z

@XiaoQuark, I left a comment on your PR that needs your attention.

here: https://github.com/CodeYourFuture/Module-Structuring-and-Testing-Data/pull/1045/changes#r2891725763

XiaoQuark · 2026-03-06T17:00:32Z

Hi @netEmmanuel, I mentioned this in my reply yesterday, but just to clarify: I didn’t remove that case. I just rewrote the test so the wording is consistent with my other tests. The “denominator equals 0” scenario is still covered in the test should return false when the denominator is equal to zero.

Let me know if you were expecting a different edge case to be tested.

favourO · 2026-03-11T23:28:16Z

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

+    throw new Error(`Invalid card format: "${card}". Expected a string.`);

  let cardSuit = card.slice(-1);
  let cardRank = card.slice(0, -1);


This looks much cleaner after removing the logs. One small thing to think about: since cardSuit and cardRank aren't reassigned later in the function, is let the most appropriate declaration here?

favourO · 2026-03-11T23:41:06Z

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


-  if (validSuits.find((suit) => suit === cardSuit)) {
-    console.log(cardSuit);
+  if (validSuits.includes(cardSuit)) {


The suit validation works well here. I’m wondering though; if the suit was invalid, what would happen to the rest of the function? Is there a way we could structure the check so that invalid cases exit earlier and the main logic stays flatter?

favourO · 2026-03-12T00:14:43Z

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

@@ -53,17 +48,16 @@ function getCardValue(card) {
      case "8":


The numeric ranks (2–10) all follow the same logic. Do you think we need a separate case for each one, or could they be handled with a single rule?

…suit and rank

XiaoQuark · 2026-03-13T13:51:42Z

Hi @favourO , thank you for your feedback. I think I have fixed everything you mentioned.
I decided to create a second array of valid ranks to validate them before entering the switch. Not sure if this was the idea you had in mind.

favourO

Great improvement, just a little more effort
I added a comment.

favourO · 2026-03-13T15:29:27Z

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

 function getCardValue(card) {
-  // TODO: Implement this function
+  const validSuits = ["♠", "♥", "♦", "♣"];
+  const validRanks = [


This is a nice improvement,
For the numeric cards, do you think you could check if the rank is between 1 and 10 instead of listing each value from 1 to 10? For example, by converting cardRank to a number and checking something like >= 1 && <= 10.

XiaoQuark added 9 commits February 23, 2026 11:01

write getAngleType function and assert tests

5a0dfa0

write jest tests for getAngleType function

f5555b2

write a series of tests for isProperFraction() function

c852657

rewrite all tests based on mathematical definition of proper fractions

ad52844

write function is ProperFraction() to pass all tests

8a750dd

refactor first test, splitting it into two, for more explicit testing…

9fd83a0

… about cases with numerator = 0

write tests for function getCardValue

9cc4680

write getCardValue() function ensuring all tests pass

e2a6053

refactor code for more descriptive error messages

8adf134

XiaoQuark added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 24, 2026

netEmmanuel 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 Mar 5, 2026

netEmmanuel suggested changes Mar 5, 2026

View reviewed changes

netEmmanuel added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Mar 5, 2026

Grajales-K added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 6, 2026

XiaoQuark mentioned this pull request Mar 9, 2026

26-Jan-ITP | Structuring and testing data | Kris Oldrini XiaoQuark/My-Coursework-Planner#48

Open

favourO reviewed Mar 12, 2026

View reviewed changes

refactor: change let variables to const and add early validation for …

faa279a

…suit and rank

favourO reviewed Mar 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

London | 26-ITP-Jan | Kris Oldrini | Sprint 3 | Implement And Rewrite Tests#1045

London | 26-ITP-Jan | Kris Oldrini | Sprint 3 | Implement And Rewrite Tests#1045
XiaoQuark wants to merge 10 commits intoCodeYourFuture:mainfrom
XiaoQuark:coursework/sprint-3

XiaoQuark commented Feb 24, 2026

Uh oh!

netEmmanuel Mar 5, 2026

Uh oh!

XiaoQuark commented Mar 5, 2026 •

edited

Loading

Uh oh!

netEmmanuel commented Mar 6, 2026 •

edited

Loading

Uh oh!

XiaoQuark commented Mar 6, 2026 •

edited

Loading

Uh oh!

favourO Mar 11, 2026

Uh oh!

favourO Mar 11, 2026

Uh oh!

favourO Mar 12, 2026

Uh oh!

XiaoQuark commented Mar 13, 2026

Uh oh!

favourO left a comment

Uh oh!

favourO Mar 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

XiaoQuark commented Feb 24, 2026

Learners, PR Template

Changelist

1 Implement solutions

2 Rewrite tests with Jest

Uh oh!

netEmmanuel Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

XiaoQuark commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netEmmanuel commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

XiaoQuark commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

favourO Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

favourO Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

favourO Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

XiaoQuark commented Mar 13, 2026

Uh oh!

favourO left a comment

Choose a reason for hiding this comment

Uh oh!

favourO Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

XiaoQuark commented Mar 5, 2026 •

edited

Loading

netEmmanuel commented Mar 6, 2026 •

edited

Loading

XiaoQuark commented Mar 6, 2026 •

edited

Loading