NW | ITP-May-25 | Geraldine Edwards | Module-Structuring-and-Testing-Data | Sprint-3 by Geraldine-Edwards · Pull Request #541 · CodeYourFuture/Module-Structuring-and-Testing-Data

Geraldine-Edwards · 2025-06-13T18:46:30Z

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

This PR includes my responses and code to questions in the Key Implement, Mandatory Rewrite, Mandatory Practice and the Stretch Investigate files.

Questions

Ask any questions you have for your reviewer.

…h section to test for other scenarios

…ck of cards

…assword validator

CameronDowner

Really strong coursework - you are using some advanced practises here (e.g. parameterised tests, breaking down functions, throwing & testing for errors)

I have left some more advanced questions, but will mark as Complete because you've met all the criteria already

CameronDowner · 2025-06-14T13:22:15Z

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

+  if (Math.abs(numerator) < Math.abs(denominator)) return true;
+  return false; //if the inputs don't satisfy the expression it returns false


Do we need an if statement here?

Math.abs(numerator) < Math.abs(denominator) evaluates to true or false already

Ah yes, I can see now that I can just return the expression instead of using the if statement - I'll fix this now. :)

CameronDowner · 2025-06-14T13:25:50Z

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

 function getCardValue(card) {
-    if (rank === "A") return 11;
+  // use an array to hold the values of the face cards.
+  const faceCards = ["10", "J", "Q", "K"];


This will work as expected - but is 10 a face card?

If I wanted to extend this functionality & used this array in another method isFaceCard, this would start giving me the wrong answer

oh gosh of course! I think I got confused as I was just thinking about the values of the cards being 10. I've removed 10 from the array and adjusted the following if statement to cover numbers 2-10.

CameronDowner · 2025-06-14T13:26:43Z

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


 // Case 2: Identify Improper Fractions:
+test("should return true for a improper fraction", () => {
+  expect(isProperFraction(5, 2)).toEqual(false);


Jest also has .toBeTruthy(): https://jestjs.io/docs/expect#tobetruthy

Why might that not be a good thing to use here?

I imagine it's not great to use at this test probably because if something is "truthy" it is less strict.. or something to that effect? I would need to make sure that an improper fraction was handled strictly to get the exact correct values.

That is exactly it - truthy values are a lot more than true which means it is less strict than .toEqual(true). It's any value that would be coerced into true when in a boolean context

https://developer.mozilla.org/en-US/docs/Glossary/Truthy

Boolean("hello") // true Boolean([]) // true Boolean({}) // true

CameronDowner · 2025-06-14T13:28:34Z

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

+  expect(aceCard).toEqual(11);
+});
+// Case 5: Handle Invalid Cards - using an array for the inputs :
+test("should throw an error for invalid card ranks", () => {


Great parameterised tests - Jest actually has functionality built-in to support this

https://jestjs.io/docs/api#testeachtablename-fn-timeout

Why might we use Jest's version over 'hand-rolling' our own parameterised test like?

I now understand that we can use Jest's .each() for clearer test output, better reporting, and less boilerplate code. In the description of the test we can use placeholders such as %s (string value) and %d (number , or digit, value), etc, as placeholders for the actual test cases inputs, meaning that when the error message appears we can immediately know which case it refers to. E.g if the current input is "G♥", the test name will be "should throw an error for invalid card rank: G♥" which makes for more readable tests.

That's exactly it - it avoids boilerplate & it's super easy to see which case has failed

CameronDowner · 2025-06-14T13:30:49Z

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

+  if (n % 100 === 11 || n % 100 === 12 || n % 100 === 13) {
+    return n + "th";
+  }
+  if (n % 10 === 1) {
+    return n + "st";
+  }
+  if (n % 10 === 2) {
+    return n + "nd";
+  }
+  if (n % 10 === 3) {
+    return n + "rd";
+  }
+  return n + "th"; //everything else in numbers
 }


We have a lot of common values here - could we extract common variables?

Naming variables can give a value meaning which make it easier to read

Ah, yes of course I could use variable names called lastDigit and lastTwoDigits, so that :
const lastDigit = n & 10;
const lastTwoDigits = n % 100;
and then adjust each if statement, for example, if (lastDigit === 1){}, etc

CameronDowner · 2025-06-14T13:31:37Z

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

+  for (const input of invalidInputs) {
+    expect(() => getOrdinalNumber(input)).toThrow("Invalid ordinal number");
+  }


This is a really great test & something you'd expect to see in industry when reviewing tests

CameronDowner · 2025-06-14T13:32:06Z

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

+  if (count === 0) {
+    return "";
+  }


What happens if we pass 0 into str.repeat?

Oh I see, yes of course ,it will return " " and empty string anyway so i don't need to account for if an input is exactly 0. oops! :)

CameronDowner · 2025-06-14T13:32:31Z

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

+test("should return the message 'Count is an invalid input'", () => {
+  const str = "hello";
+  const count = "three";
+  expect(() => repeat(str, count)).toThrow("Count is an invalid input");


Why do we pass a function here instead of just calling repeat(str, count)?

When I was looking at this I read about how if we didn't pass the arrow function to "expect", then Jest won't be able to catch the error to check it, if it was just calling repeat(str, count) it would run immediately before Jest can even check it and likely crash the program.

CameronDowner · 2025-06-14T13:33:40Z

Sprint-3/4-stretch-investigate/card-validator.js

@@ -0,0 +1,90 @@
+/*
+ Helper function to check if the card number is a string using the typeof operator.
+ Because credit card numbers can start with zero, using a "number" type would mean dropping any leading zeros which would result in loss of information.


Breaking down functions like this is amazing to see

It makes the code really easy to read and reason about. You can forget the specifics of the function and trust it does what the function is called.

CameronDowner · 2025-06-14T13:34:26Z

Sprint-3/4-stretch-investigate/card-validator.js

+function isValidCardNumber(cardNumber) {
+  //if the card number input is not a "string" type.
+  if (!isString(cardNumber)) {
+    return "Card number must be a string";


Could these be errors instead of strings?

Yes I understand, throwing an JS error is more standard procedure for invalid inputs rather than relying on a bespoke message, I'll change these. :)

…ber 10

…eadable

Gerbil1511 added 8 commits June 9, 2025 18:34

add code to getAngleType function for testing, add additional test cases

f464538

add additional acceptance criteria for the tests, complete the stretc…

5a452a0

…h section to test for other scenarios

update original function and add test cases to check the rank of a pa…

5724979

…ck of cards

add jest tests to 1-key-implement functions

fe3ff3c

update functions in files and add tests for certain conditions.

5167a24

create function to validate a credit card number and add test file

f2fe186

add answers to questions about the find function

826e1db

add tests (using an array for the inputs) and complete function for p…

e621b0b

…assword validator

Geraldine-Edwards added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jun 13, 2025

CameronDowner reviewed Jun 14, 2025

View reviewed changes

CameronDowner added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jun 14, 2025

Geraldine-Edwards mentioned this pull request Jun 17, 2025

Step 2 - Structuring and Testing Data Submission Geraldine-Edwards/CYF-Coursework-Planner#95

Open

Gerbil1511 added 6 commits June 17, 2025 15:46

adjust return statement in isProperFraction function

be02da1

correct faceCards array and following if statement to handle card num…

becf97a

…ber 10

replace hand-rolled parameterised test with Jest's .each() version

f7d09da

extract the common values into well named variables so code is more r…

6bb7c11

…eadable

adjust function repeat(str,count) for clarity

32d6f56

switch invalid inputs from returning messages to 'throw error' messages

5fe320b

illicitonion closed this Sep 1, 2025

		if (Math.abs(numerator) < Math.abs(denominator)) return true;
		return false; //if the inputs don't satisfy the expression it returns false

Uh oh!

Conversation

Geraldine-Edwards commented Jun 13, 2025

Learners, PR Template

Changelist

Questions

Uh oh!

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

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!

Geraldine-Edwards Jun 18, 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!

CameronDowner Jun 14, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Geraldine-Edwards Jun 18, 2025 •

edited

Loading

CameronDowner Jun 14, 2025 •

edited

Loading