West Midlands | ITP-Sept-25 | Mustaf Asani | Sprint 3 | Coursework/sprint 3 implement and rewrite #795

asaniDev · 2025-10-23T00:49:29Z

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

created functions and practiced test driven development by making tests for the functions.

github-actions · 2025-10-23T00:49:47Z

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Sprint part (Sprint-3) doesn't match expected format (example: 'Sprint 2', without quotes)

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

The files in the Sprint-2 folder are not related to Sprint-3 exercise. Can you revert the changes made in the Sprint-2 folder?

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

cjyuan · 2025-10-27T01:29:11Z

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

 // just make one change at a time -- don't rush -- programmers are deep and careful thinkers
 function getCardValue(card) {
-  if (rank === "A") {
+  const rank = card[0];


card[0] yields only a string with the first character -- not necessary the whole rank.

thank you for pointing this out, I do need to make my tests more robust. I managed to get the correct rank for single digit and multiple digit cards.

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

cjyuan · 2025-10-27T01:36:47Z

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

+test("should return true for a proper fraction based on the absolute values of the numerator and denominator", () => {
+  expect(isProperFraction(-3, 8)).toEqual(true);
+});


To make the test more comprehensive, we could test multiple samples like this:

test("should return true for a proper fraction based on the absolute values of the numerator and denominator", () => { expect(isProperFraction(-3, 8)).toEqual(true); expect(isProperFraction(-3, -8)).toEqual(true); expect(isProperFraction(3, -8)).toEqual(true); expect(isProperFraction(3, 8)).toEqual(true); })

added multiple test samples for this and other lines as well.

cjyuan · 2025-10-27T01:37:53Z

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

+test("should return 11 for Ace of Spades", () => {
+  const invalidCard = getCardValue("1♥");
+  expect(invalidCard).toEqual("Invalid card rank.");
+});


The test description does not quite match what is being tested.

corrected the test description.

cjyuan · 2025-10-27T01:39:35Z

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

+test("should return a number matchig the rank value for given card", () => {
+  const numberCard = getCardValue("5♠");
+  expect(numberCard).toEqual(5);
+});


I think "... for number cards (2-10)" would be more informative than "... for given card")

Could test multiple values, especially the boundary cases like "2♠" and "10♠".

added a more informative description. also tested the edge cases.

…orrected test description for invalid cards, used more informative descriptions

… for cards with more than 1 digit, removed check for negative value since im using maths.abs

cjyuan

There are still three modified files in the "Sprint-2" folder. Can you revert the changes made to them?

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

cjyuan · 2025-10-29T00:59:53Z

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

+  let rank = "";

-  if (card === "A♠") {
-    return 11;
-  } else if (Number(rank) > 1 && Number(rank) < 10) {
+  for (let i = 0; i < card.length - 1; i++) {
+    rank += card[i];
+  }


This approach work.

You could also consider using built-in functions (methods) of String. Mastering these String's methods can simplify a lot of string processing tasks.

cjyuan · 2025-10-29T01:01:08Z

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

+  if (Number.isInteger(Number(rank)) && Number(rank) > 1 && Number(rank) < 10) {
    return Number(rank);


In JavaScript, strings that represent valid numeric literals in the language can be safely
converted to equivalent numbers or parsed into a valid integers.
Do you want to recognize these string values as valid ranks?

To find out what these strings are, you can ask AI

What kinds of string values would make Number(rank) evaluate to 2 in JS?

i added a regex to only allow specific numeric literals that convert to numbers which would be in our range 2 -10 and ignore all others.

cjyuan · 2025-10-29T01:03:04Z

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

  expect(isProperFraction(7, 4)).toEqual(false);
+  expect(isProperFraction(35, 8)).toEqual(false);


Could also test some negative improper fractions in this test.

cjyuan · 2025-10-29T01:03:40Z

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

 test("should return 10 for Face Cards", () => {
-  const faceCard = getCardValue("K♠");
+  const faceCard = getCardValue("10♠");


Face cards are J, Q, K. 10 is consider a number card.

cjyuan · 2025-10-29T01:04:38Z

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

  const numberCard = getCardValue("5♠");
  expect(numberCard).toEqual(5);


We could test multiple samples (especially the boundary cases, "2♠" and "10♠") to make the test more robust.

added a suite of tests for different values to make the test more robust

cjyuan · 2025-10-29T01:06:32Z

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

+test("should return invalid card rank for cards that are not in the suite", () => {
+  const invalidCard = getCardValue("100♥");
+  expect(invalidCard).toEqual("Invalid card rank.");
+});
+
+test("should return invalid card rank for cards that are not in the suite", () => {
+  const invalidCard = getCardValue("3.1416♥");
  expect(invalidCard).toEqual("Invalid card rank.");
 });


They both test "invalid case". Consider combine them into one test but using multiple samples.

combined the tests and added more tests for different values

…nnecessary if statements

…be converted to numbers.

…mber cards test suite.

cjyuan

The tests are now very solid. Well done.
This PR branch still contains unrelated changes in "Sprint-2" folder. Can you revert the changes made in the "Sprint-2" folder to make the PR branch clean?

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

cjyuan

Changes look good. Well done.

cjyuan · 2025-11-22T23:57:20Z

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

+  if (strictNumber(rank)) {
+    if (pattern.test(Number(rank))) {
+      return Number(rank);
+    }
+  }


Is the outer if statement necessary?

cjyuan · 2025-11-23T00:00:07Z

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

+
+  if (faceCardPattern.test(rank)) {
+    return 10;
+  } else if (/^A$/.test(rank)) {


Could also just write rank == 'A'.

asaniDev added 16 commits October 14, 2025 00:23

changed the name of the string in the function

26fd354

removed line 10 and passed function to console

5ab6322

corrected the error with the function

0c7cc6f

added a return to the function

cc216e1

moved the return command to the line below it

90013dc

created a function that returns the bmi

f63f7cf

created a function that converts a given string to snake case

a02b0c5

created a toPounds function to return a pounds and pence notation

e097316

answered all the questions in the exercise

742c228

wrote tests for any edge cases and fixed resulting bugs

0d5ad45

made a function for angle types and added tests for all angles

2038048

added tests for most fraction types and conditions for them as well

7daf16b

added conditions for all ranks in a deck of cards and tests for them

dfebb97

added jest tests for all angle types

6865348

added jest tests for different card values

43d650c

took away comment on line 30

3ca95c6

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

cjyuan reviewed Oct 27, 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 27, 2025

asaniDev added 3 commits October 27, 2025 14:07

revert multiple commits that were from sprint 2

c56bd67

added tests for multiple samples, removed a test for ace of spades, c…

2765c49

…orrected test description for invalid cards, used more informative descriptions

removed check for ace of spades, adjusted rank to give the full value…

035e27e

… for cards with more than 1 digit, removed check for negative value since im using maths.abs

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

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

asaniDev added 2 commits November 17, 2025 02:59

corrected test for numerator is 0 to a proper fraction, and removed u…

50ee9ef

…nnecessary if statements

added a regex to make sure only exact number card value matches will …

bcbd867

…be converted to numbers.

asaniDev added 3 commits November 17, 2025 12:32

added a few tests for negative improper fractions

137fd7c

added more tests to make the test suites more robust, moved ten to nu…

e5b18e4

…mber cards test suite.

removed console.log used for testing.

5fa3d08

asaniDev added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Nov 17, 2025

cjyuan reviewed Nov 17, 2025

View reviewed changes

Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js Outdated Show resolved Hide resolved

Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js Outdated Show resolved Hide resolved

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

asaniDev added 6 commits November 20, 2025 02:49

used regex pattern to check for ranks and removed includes method

d58910a

added dev dependencies

8b52886

revert changes to sprint 2 folder

78f21df

reverted changes to 1.js

8ad9d08

reverted changes to 2.js in sprint 2 file.

14ddfad

revert unwanted change

1a5aada

asaniDev added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Nov 22, 2025

cjyuan reviewed Nov 23, 2025

View reviewed changes

		if (Number.isInteger(Number(rank)) && Number(rank) > 1 && Number(rank) < 10) {
		return Number(rank);

		expect(isProperFraction(7, 4)).toEqual(false);
		expect(isProperFraction(35, 8)).toEqual(false);

		const numberCard = getCardValue("5♠");
		expect(numberCard).toEqual(5);

Uh oh!

West Midlands | ITP-Sept-25 | Mustaf Asani | Sprint 3 | Coursework/sprint 3 implement and rewrite #795

Are you sure you want to change the base?

West Midlands | ITP-Sept-25 | Mustaf Asani | Sprint 3 | Coursework/sprint 3 implement and rewrite #795

Uh oh!

Conversation

asaniDev commented Oct 23, 2025

Learners, PR Template

Changelist

Uh oh!

github-actions bot commented Oct 23, 2025

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cjyuan Oct 27, 2025 • 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!

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!

cjyuan left a comment

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!

Choose a reason for hiding this comment

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cjyuan Oct 27, 2025 •

edited

Loading