Sheffield | May-2025 | Hassan Osman | Sprint-3 #580

HassanOHOsman · 2025-06-20T15:05:24Z

Learners, PR Template

Self checklist

I have committed my files one by one, on purpose, and for a reason
I have titled my PR with REGION | COHORT_NAME | FIRST_NAME LAST_NAME | PROJ_NAME
I have tested my changes
My changes follow the style guide
My changes meet the requirements of this task

Changelist

In this PR, I implemented the functions and tested them using Jest.

Questions

Ask any questions you have for your reviewer.

cjyuan · 2025-06-27T17:06:57Z

Sprint-3/2-mandatory-rewrite/1-get-angle-type.js

@@ -1,7 +1,10 @@
 function getAngleType(angle) {
  if (angle === 90) return "Right angle";
-  // replace with your completed function from key-implement
-
+  if (angle < 90) return "Acute angle";


angle <= 0 should also be considered as invalid angle.

cjyuan · 2025-06-27T17:08:47Z

Sprint-3/2-mandatory-rewrite/1-get-angle-type.test.js

 // Case 5: Identify Reflex Angles:
 // When the angle is greater than 180 degrees and less than 360 degrees,
 // Then the function should return "Reflex angle"
+
+
+test("should identify angle over 360", () => {


We could generalise this test to "should identify angle not in the range (0, 360) as 'invalid'")

cjyuan · 2025-06-27T17:13:59Z

Sprint-3/2-mandatory-rewrite/2-is-proper-fraction.js

+    if (numerator < denominator && (numerator/denominator) > 0) return true;
+    if (numerator > denominator) return false;
+    if (numerator < denominator && (numerator/denominator) < 0) return "Negative Fraction";
+    if (numerator === denominator ) return "Not really a fraction";
+


The function should return either true or false.

true means "the given numerator and denominator can form a proper fraction".

false means "the given numerator and denominator cannot form a proper fraction".

Note: If a function can possibly return different types of value, it would make consuming the return value difficult.

In mathematics, -4/7 == 4/-7, and -4/-7 == 4/7.
So, ideally isProperFraction() should recognise all of them as proper fractions.

Hint: If you compute the absolute value of both parameters inside the function first, the code can become much simpler.

That's totally ture CJ! I kept getting errors for some reason during tests, hence the multiple unnecessary scenarios I added. Anyway, function is now back to a simpler form.

cjyuan · 2025-06-27T17:15:33Z

Sprint-3/2-mandatory-rewrite/2-is-proper-fraction.test.js

+test("should return true for an improper fraction", () => {
+  expect(isProperFraction(11, 3)).toEqual(false);
+  expect(isProperFraction(7, 2)).toEqual(false);
+});


The test description does not quite match the tests being performed.

silly me! lol 😅 Changed this now

cjyuan · 2025-06-27T17:16:28Z

Sprint-3/2-mandatory-rewrite/2-is-proper-fraction.test.js

+test("should return a negative fraction", () => {
+  expect(isProperFraction(-11, 3)).toEqual("Negative Fraction");
+});
+
+
 // Case 4: Identify Equal Numerator and Denominator:
+
+test("should return a whole number", () => {
+  expect(isProperFraction(4, 4)).toEqual("Not really a fraction");
+});


After you have updated the function implementation, you may want to update these tests accordingly.

cjyuan · 2025-06-27T17:25:27Z

Sprint-3/2-mandatory-rewrite/3-get-card-value.js

+    if (card === "Ace of Spades") return 11;
+    if (card === "Number Cards") return "2-10";
+    if (card === "Ace") return "A";
+    if (card === "Face Cards") return "J, Q, K";


You probably missed the spec of this function in Sprint-3/1-key-implement.

The expected parameter is a string in this format: "A♠", "2♠", "10♥", "K♥", "Q♦", "J♣". The rank value can be an invalid string but the last character is always a valid suit character.

getCardValue("A♠") should return 11.
getCardValue("K♠") should return 10.
getCardValue("2♠") should return 2.

cjyuan · 2025-06-27T17:29:47Z

Sprint-3/3-mandatory-practice/implement/count.js

+    if (str.includes(char) === true) {
+        return str.split(char).length -1;
+    } else {
+        return 0;
+    }


What is the value of str.split(char).length when str does not contain any character matching the value of char?
Can we utilise the finding from the previous question to simplify our code?

Simplified now.

cjyuan · 2025-06-27T17:33:00Z

Sprint-3/3-mandatory-practice/implement/get-ordinal-number.js

+    if (num === 1) {
+        return "1st";
+    } else if(num === 21) {
+        return "21st";
+    } else if(num === 2) {
+        return "2nd";
+    } else if(num === 22) {
+        return "22nd";
+    } else {
+        return `${num}th`;


getOrdinalNumber(101) should return "101st".
Consider looking up the rules to clarify how ordinal numbers are formed.

function has now been changed to accommodate all numbers.

cjyuan · 2025-06-27T17:34:01Z

Sprint-3/3-mandatory-practice/implement/repeat.js

+    if(count > 0) {
+        return str.repeat(count); 
+    } else if (count === 0) {
+        return " ";


" " is a string containing one space. "" is an empty string (a string containing 0 characters).

cjyuan · 2025-06-30T13:43:37Z

Sprint-3/2-mandatory-rewrite/2-is-proper-fraction.js

@@ -1,6 +1,10 @@
 function isProperFraction(numerator, denominator) {
-    if (numerator < denominator) return true;
-    // add your completed function from key-implement here
+    if (numerator >= denominator) {


A single comparison is enough if we can ensure the values being compared are positives.

That is, treat -2/3, 2/-3, and -2/-3 the same as 2/3. (They are all proper fractions).

added Math.abs() in the hope it would sort out the issue.

cjyuan · 2025-06-30T13:47:55Z

Sprint-3/2-mandatory-rewrite/3-get-card-value.js

+            return 11;
+            break;
+        case "2♠":
+            return 2;


A card can have one of four possible suits; the last character can also be a heart, club, or diamond. Your approach works great if you can extract the rank from the parameter first.

cjyuan · 2025-06-30T13:52:23Z

Sprint-3/2-mandatory-rewrite/3-get-card-value.test.js

 // Case 3: Handle Face Cards (J, Q, K):
+test("should return 10 for K♠ card", () => {


We could generalise this test to "should return 10 for face cards (J, Q, K)" and check all three ranks J, Q, K). Same for "all number cards".

Sprint-3/3-mandatory-practice/implement/get-ordinal-number.test.js

Sprint-3/3-mandatory-practice/implement/repeat.js

cjyuan

Changes look good.

Sprint-3/3-mandatory-practice/implement/get-ordinal-number.test.js could still be improved.

cjyuan · 2025-07-01T07:32:49Z

Sprint-3/2-mandatory-rewrite/3-get-card-value.test.js

+test("should return 10 for face cards (J, Q, K)", () => {
+    const faceCards = getCardValue("K♣︎");
    expect(faceCards).toEqual(10);
 });

-test("should return 10 for Q♠ card", () => {
+test("should return 10 for face cards (J, Q, K)", () => {
    const faceCards = getCardValue("Q♠");
    expect(faceCards).toEqual(10);
 });


We could combine these tests as

test("should return 10 for face cards (J, Q, K)", () => { expect(getCardValue("J♣︎")).toEqual(10); expect(getCardValue("Q♠")).toEqual(10); expect(getCardValue("K♣︎")).toEqual(10); });

cjyuan · 2025-07-01T07:38:54Z

Sprint-3/3-mandatory-practice/implement/get-ordinal-number.test.js

I don't think your implementation can pass some of the tests in this script. Have you tried executing this script to test your function implementation?

Yes, some mistakes in the 2 expected values which have now been corrected.

cjyuan · 2025-07-01T07:41:24Z

Sprint-3/3-mandatory-practice/implement/get-ordinal-number.test.js

+test("should append 'st' to numbers ending with 1 and not ending with 11", () => {
    expect(getOrdinalNumber(1)).toEqual("1st");
    });

+test("should append 'st' to numbers ending with 1 and not ending with 11", () => {
+    expect(getOrdinalNumber(41)).toEqual("41st");
+    });
+


Can be combined.

HassanOHOsman · 2025-07-01T13:55:46Z

CJ, is there a way I could jests test a specific file, rather than running the whole test for all files? Tried doing so but keep ending up with errors :(

cjyuan · 2025-07-01T14:03:56Z

Changes are good! Good job.

You can run jest on a single file as
npx jest <filename> (you might need to run this command in the folder where the test script is located)

HassanOHOsman added 5 commits June 18, 2025 15:13

implemented & tested function for all angle types

9b98e48

tested & implemented different card types

1e65655

implemented and tested the char count

5634292

implemented and tested ordinal number function

5d17a56

implemented and tested the repeat function

531b3ed

HassanOHOsman added Needs Review Participant to add when requesting review 📅 Sprint 3 Assigned during Sprint 3 of this module labels Jun 20, 2025

cjyuan reviewed Jun 27, 2025

View reviewed changes

cjyuan added Reviewed Volunteer to add when completing a review and removed Needs Review Participant to add when requesting review labels Jun 27, 2025

HassanOHOsman added 8 commits June 28, 2025 14:18

changed to make angles <=0 evaluates to "invalid"

1472a6e

updated the last test description for more clarity.

0286d78

simplified the conditions of my function

5bd333e

updated function implementation & tests

3498122

updated card function implementation & test

33809e8

changed ordinal number function to widen range

f8895ad

corrected my empty string return

06411c8

simplified my function

489b22f

HassanOHOsman added Needs Review Participant to add when requesting review and removed Reviewed Volunteer to add when completing a review labels Jun 29, 2025

cjyuan reviewed Jun 30, 2025

View reviewed changes

cjyuan added Reviewed Volunteer to add when completing a review and removed Needs Review Participant to add when requesting review labels Jun 30, 2025

HassanOHOsman added 5 commits June 30, 2025 19:32

added Math.abs()

6284562

edited function to cover different suits & ranks

c4a2aca

updated tests descriptions

7c4389d

updated tests descriptions

499bcf1

updated the error message

34dd33f

HassanOHOsman added Needs Review Participant to add when requesting review and removed Reviewed Volunteer to add when completing a review labels Jun 30, 2025

cjyuan reviewed Jul 1, 2025

View reviewed changes

cjyuan added Reviewed Volunteer to add when completing a review and removed Needs Review Participant to add when requesting review labels Jul 1, 2025

HassanOHOsman added 5 commits July 1, 2025 14:18

refactored Face Cards(J,Q,K) test further

784a678

refactored tests for (2-10) cards further

0ec98ea

corrected mistakes in a few of ordinal num tests

f7bfacf

removed a bug from ordinal number'd function

8461178

further refactored ordinal numbers tests

16a010c

HassanOHOsman added Needs Review Participant to add when requesting review and removed Reviewed Volunteer to add when completing a review labels Jul 1, 2025

cjyuan added Complete Volunteer to add when work is complete and review comments have been addressed and removed Needs Review Participant to add when requesting review labels Jul 1, 2025

HassanOHOsman mentioned this pull request Jul 1, 2025

May-2025 | Structuring and testing data | Hassan osman HassanOHOsman/My-Coursework-Planner#53

Open

		// Case 3: Handle Face Cards (J, Q, K):
		test("should return 10 for K♠ card", () => {

Uh oh!

Sheffield | May-2025 | Hassan Osman | Sprint-3 #580

Are you sure you want to change the base?

Sheffield | May-2025 | Hassan Osman | Sprint-3 #580

Uh oh!

Conversation

HassanOHOsman commented Jun 20, 2025

Learners, PR Template

Changelist

Questions

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!

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!

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!

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 Jul 1, 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!

HassanOHOsman commented Jul 1, 2025

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

cjyuan Jul 1, 2025 •

edited

Loading

cjyuan commented Jul 1, 2025 •

edited

Loading