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?

cjyuan · 2025-10-27T01:25:09Z

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

 // write one test at a time, and make it pass, build your solution up methodically

 function isProperFraction(numerator, denominator) {
+  if (Math.sign(numerator) === -1 || Math.sign(denominator) === -1) {


There is an easier syntax to check if a number is negative.

And since you are using Math.abs(), checking for negatives is optional.

This function can be further simplified. If you are up to the challenge, go for a solution that uses at most one if-statement.

removed the check for negative and took off the first if else block, not quite sure how to reduce it to one if statement without making risky assumptions. since the function only returns true for a proper fraction and false for everything else, would it be appropriate to have an if statement that checks for a proper fraction and else that gives false for any other scenario?

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:34:36Z

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

+  } else if (Number(rank) > 1 && Number(rank) < 10) {
+    return Number(rank);
+  } else if (rank === "10" || rank === "J" || rank === "Q" || rank === "K") {


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

getCardValue("10♠"); getCardValue("100♠"); getCardValue("3.1416♠");

the last function with 3.1416 showed that my condition for number cards was not checking that we have only a positive integer. I added an extra rule to check for an integer that way any decimal vlues will be flagged as invalid.

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?

cjyuan · 2025-10-29T00:56:26Z

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

+  if (Math.abs(numerator) < Math.abs(denominator)) {
    return true;
  } else if (numerator > denominator) {
    return false;
  } else if (numerator === denominator) {
    return false;
+  } else if (numerator === 0) {
+    return false;


This is a good opportunities to practice code tracing -- to evaluate how the code get executed step by step using some sample pairs of parameter values. For examples,

-1, 2

1, 2

1, 0

1, 1

0, 1

Hopefully you can figure out why some of your code is not needed.

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?

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.

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.

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

		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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

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!

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

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

cjyuan Oct 27, 2025 •

edited

Loading

cjyuan Oct 29, 2025 •

edited

Loading