LONDON-JAN-25 | ANDREI FILIPPOV | Module-Structuring-and-Testing-Data | WEEK 3 by Droid-An · Pull Request #309 · CodeYourFuture/Module-Structuring-and-Testing-Data

Droid-An · 2025-02-20T11:28:54Z

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 COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
I have tested my changes
My changes follow the style guide
My changes meet the requirements of this task

Changelist

Complete Sprint 3 coursework

Questions

no questions

ChrisCookOC

You've made good progress here, seems you are getting the hang of unit testing and using jest which is great to see :)

ChrisCookOC · 2025-03-05T16:19:25Z

Sprint-3/1-key-implement/2-is-proper-fraction.js


 function isProperFraction(numerator, denominator) {
-    if (numerator < denominator) return true;
+    if (Math.abs(numerator) < Math.abs(denominator)) return true;


This all works, using the if / else if / else if.
However, a nice thing here is that your return values are booleans, which means you should be able to simplify this down into just two checks

Can you see a way of rewriting this in a simpler manner?

Can give a hint if needed! :)

Oh, now I see how I can simplify that. I just removed last two conditions which are unnecessary

Yep exactly :)

ChrisCookOC · 2025-03-05T16:20:26Z

Sprint-3/1-key-implement/2-is-proper-fraction.js

 // Stretch:
 // What other scenarios could you test for?
+
+const improperNegativeFraction = isProperFraction(-5, 2);


Good scenario to test for!

ChrisCookOC · 2025-03-05T16:23:28Z

Sprint-3/1-key-implement/3-get-card-value.js

 // When the function is called with such a card,
 // Then it should return the numeric value corresponding to the rank (e.g., "5" should return 5).
-const fiveofHearts = getCardValue("5♥");
+const fiveOfHearts = getCardValue("2♥");


I assume this comes from changing it to different values to test it is working, which is a great thing to do.

Just be careful when you are committing because the name of the variable is now misleading!

Yes, you're right. I was just short on time to check the correct names for the card values. I've updated it now.

ChrisCookOC · 2025-03-05T16:39:49Z

Sprint-3/1-key-implement/3-get-card-value.js

+  if (rank === "A") return 11;
+  else if (rank === "J" || rank === "Q" || rank === "K") return 10;
+  else if (
+    typeof Number(rank) == "number" &&


This works, but are you able to find any functions you can call that check if something is a number instead? If they exist it is often best to use them and avoid having to code type checks like this!

Yes, I found the isNaN function, which seems to do exactly that.

ChrisCookOC · 2025-03-05T16:42:08Z

Sprint-3/1-key-implement/3-get-card-value.js

 // Given a card with an invalid rank (neither a number nor a recognized face card),
 // When the function is called with such a card,
 // Then it should throw an error indicating "Invalid card rank."
+const invalid = getCardValue("five of hearts");


What should happens if I give you the input of "5.123♥"? Is that reflected in the program?

Thank you for pointing this out! I hadn’t considered that case. I resolved it by adding an isInteger condition.

ChrisCookOC · 2025-03-05T16:56:23Z

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

 function getOrdinalNumber(num) {
- return "1st";
+  numToString = String(num);
+  if (numToString[numToString.length - 1] == 1 && num != 11) {


You repeat this multiple times
numToString[numToString.length - 1]

Can you see the benefits of doing this array access only once? if so, how would you do it?

Yes, I can assign that value to the lastDigit variable and use that variable throughout the code.

ChrisCookOC

Thanks for taking the time to make some more changes :) Looks good.

ChrisCookOC · 2025-03-07T18:23:46Z

Sprint-3/1-key-implement/3-get-card-value.js

+  else if (rank === "J" || rank === "Q" || rank === "K") return 10;
+  else if (
+    !isNaN(rank) &&
+    Number.isInteger(+(rank)) &&


Nice finds. Something you might be able to play with if you have time. Do you need both checks? Trial and error and find out :)

Oh, thank you for the remark. Testing showed that !isNaN is not necessary here.
Article about isInteger on MDN says:

If the value is NaN, return false.

So isInteger do job of both checks.

Droid-An and others added 6 commits February 18, 2025 13:13

done implement section

30382f5

added package-lock.json to git ignore

b4796c5

fixed get-angle-type in implement section

4214ebf

done first task in rewrite section

fdd426e

done rewrite section

0efcab3

done implement section

0938167

Droid-An added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 20, 2025

Droid-An added 2 commits February 22, 2025 13:38

removed unnecessary console.log commands

dd11749

ignore it

412a244

ChrisCookOC added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Mar 5, 2025

ChrisCookOC reviewed Mar 5, 2025

View reviewed changes

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

Droid-An added 3 commits March 5, 2025 19:59

refactored isProperFraction

458b508

refactored get card value function

822c3aa

refactored get ordinal number function

4d5092e

Droid-An added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Mar 5, 2025

Droid-An requested a review from ChrisCookOC March 5, 2025 22:26

ChrisCookOC reviewed Mar 7, 2025

View reviewed changes

ChrisCookOC 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 7, 2025

deleted unnecessary check

8d1fafd

Droid-An added the Complete Volunteer to add when work is complete and all review comments have been addressed. label Mar 8, 2025

illicitonion closed this Sep 1, 2025

Uh oh!

Conversation

Droid-An commented Feb 20, 2025

Learners, PR Template

Changelist

Questions

Uh oh!

ChrisCookOC 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!

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!

ChrisCookOC 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!

3 participants