London | 25-ITP-May | Houssam Lahlah | Sprint 3 | Coursework/sprint-3 #702

HoussamLh · 2025-09-09T21:55:27Z

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

Understanding how to validate ranks strictly (not accepting "0002" or "0x02" as valid numbers).

Deciding whether to use explicit checks vs regex for number cards.

Finding the balance between writing enough tests vs not writing 52 individual card tests.

Making tests more generalised (by category) instead of repetitive.

Questions

Is my regex approach (/^[2-9]$/) the right level of strictness for checking number cards, or would you prefer a different method?
For the tests, is it better to keep them category-based with a few examples (as I’ve done), or would it be valuable to also include a loop over all possible ranks (2–10, J, Q, K, A)?
Should I also test different suits systematically (♠, ♥, ♦, ♣), or is one suit per category enough since the suit doesn’t affect value?
Is the error handling message "Invalid card rank" clear enough, or would you recommend more specific errors (e.g., "Unsupported card format" vs "Invalid rank")?
Would using a map/dictionary (e.g., {A:11, J:10, Q:10, K:10}) for ranks be cleaner than multiple if statements, or is the current approach acceptable for this task?

…dle, and last names.

…om file path string using slice.

…ween minimum and maximum.

…tion.

Added comments to exclude instructional lines from execution.

Updated age variable to let to allow incrementing its value

Declare variable before using in template string.

Add comments explaining slice error and fix by converting number to string

…umerator cases and add stretch scenarios for isProperFraction edge case testing

…id cards and throw error for invalid ones

…ractions

…e cards; throw error for invalid cards

…alid input

… string

- Return proper suffixes for 1st, 2nd, 3rd, and others (e.g., 4th, 11th, 23rd) - Added Jest tests for various edge cases including teens (11th-13th) - Improved function to handle general inputs dynamically

- Added str and count parameters to repeat function - Used String.repeat to repeat str count times - Throws error if count is negative - Added tests for count = 1, 0, and negative values

…iled comments and explanations

- Updated 3-get-card-value.js to reject invalid ranks like "0002♠", "0x02♠", and "2.1♠" - Simplified logic with explicit checks for A, J, Q, K, 10, and digits 2–9 - Updated 3-get-card-value.test.js to group tests by categories: • Ace → returns 11 • Number cards (2–10) → return their numeric value • Face cards (J, Q, K) → return 10 • Invalid ranks → throw "Invalid card rank"

…dule-Structuring-and-Testing-Data into coursework/sprint-3

- Added tests for repeat() covering: • repeating a string multiple times • returning empty string when count is 0 • throwing error when count is negative - Cleaned up unused code to follow best practices

LonMcGregor

Good start on this sprint's tasks, I have spotted a few areas where you could improve code further

A few of your files contain merge conflicts. I've left a comment on the first one for you to look at. I can't review those files further until those issues are resolved.

Great questions! I've left comments at the relevant places within your code.

LonMcGregor · 2025-09-10T12:35:14Z

Sprint-3/1-implement-and-rewrite-tests/implement/1-get-angle-type.js

@@ -8,11 +8,19 @@
 // Then, write the next test! :) Go through this process until all the cases are implemented

 function getAngleType(angle) {
+<<<<<<< HEAD:Sprint-3/1-key-implement/1-get-angle-type.js


Here, where the file contains lines marked between <<<<<< HEAD and >>>>>>> COMMIT_HASH there is a merge conflict. Do you know what this means? Can you spot the problem that will happen if you commit files containing conflicts?

hello LonMcGregor,

Thank you for your feedback and I'm sorry about the conflicts marks. I should not commit files with these markers because the code will break:

The <<<<<<<, =======, >>>>>>> lines are not valid JavaScript.

Your program will throw syntax errors if run.

also signals that the conflict hasn’t been resolved, which can confuse your team.

LonMcGregor · 2025-09-10T12:37:32Z

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

+}
+
+
+


Did you mean to commit all these blank lines?

hello LonMcGregor,

Thank you for your feedback and I'm sorry about the blank lines. I should not commit file with all this blank lines.

LonMcGregor · 2025-09-10T12:38:27Z

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

+    throw new Error("Denominator cannot be zero");
+  }
+
+  return Math.abs(numerator) < Math.abs(denominator);


Does this correctly handle all cases? What about negative improper fractions?

Hello LonMcGregor,

Thank you for the comment!

After checking, the function already handles negative improper fractions correctly because it compares the absolute values of the numerator and denominator. This way, whether the numerator or denominator is negative, the function still correctly determines if the fraction is proper.

I also included a check for denominator === 0 to avoid division by zero.

LonMcGregor · 2025-09-10T12:45:06Z

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

+  expect(getCardValue("10♥")).toEqual(10);
+
+  // You can even loop for efficiency:
+  for (let i = 2; i <= 10; i++) {


for Question 2: When testing, it is most important to test two things: a sample of normal data and edge cases. For something simple like this you could loop over everything, but that would not be a good idea, or even be possible for a more complex task. In a comment I see you mention efficiency. what does efficiency mean here? Is repeating the same action many times (looping) efficient?

Hello LonMcGregor,

Thank you for the feedback!

I see now that looping over all number cards isn’t necessarily the most efficient approach in testing. Efficiency in this context means covering a representative sample of normal data and edge cases, so we get confidence that the function works correctly without making tests unnecessarily long or harder to debug.

I’ve revised the tests to focus on representative normal values (2, 5, 10) rather than looping over all number cards. This way we still test important cases while keeping the tests clear, easy to read, and maintainable. Edge cases like Ace, face cards, and invalid cards are still covered.

LonMcGregor · 2025-09-10T12:45:31Z

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

+  if (rank === "10") return 10;
+
+  // Explicit check for number cards (2–9 only, no leading zeros or decimals)
+  if (/^[2-9]$/.test(rank)) {


for question 1: Regex works in these cases, but you will be limited to testing a single digit, which means you need that separate condition for 10.

Hello LonMcGregor,

Thank you for the feedback!

I updated the regex to ^(?:[2-9]|10)$ so it now matches all number cards from 2–10 in a single condition. This removes the need for a separate check for "10" while still correctly handling Ace, face cards, and invalid cards.

LonMcGregor · 2025-09-10T12:46:38Z

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

+
+// Case 4: Invalid cards
+test("should throw an error for invalid card rank", () => {
+  expect(() => getCardValue("Z♣")).toThrow("Invalid card rank");


for question 3: The goal of testing is to try and cover all possible goal branches. If you have sufficient sample of possible input, possible known bad inputs, and edge cases, that's enough. Do you think you have achieved that?

Hello LonMcGregor,

Thank you for the feedback!

I’ve updated Case 4 to include a representative sample of invalid inputs, including invalid letters, decimals, leading zeros, empty strings, and suit-only inputs. This ensures the error branch is thoroughly tested and all possible invalid card scenarios are covered.

LonMcGregor · 2025-09-10T12:47:58Z

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

+    return Number(rank);
+  }
+
+  throw new Error("Invalid card rank");


for question 4: The goals of error messages are to inform the user of some code when a problem occurs. If you were using this code and you saw this error, would you understand what it meant? If so, the error is good enough.

Hello LonMcGregor,

Thanks for the feedback!

I see that error messages should be clear to the user. I could improve this one by including the invalid card value in the message, so it’s immediately obvious what caused the error and makes debugging easier.

LonMcGregor · 2025-09-10T12:49:47Z

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

+
+  if (!rank) throw new Error("Invalid card rank");
+
+  if (rank === "A") return 11;


for question 5: a map/dict would also work, but might be a bit overly complicated for this task. My personal preference is if something can be done in 1-3 lines that's usually sufficient. If you had many more possible options, like if each of the 52 cards in the pack had a different rank, then you might consider a different structure. But this is fine.

Hello LonMcGregor,

Thanks! I agree — for a small set of card ranks, the current approach is simple and readable. Using a map would work, but for this task it would be unnecessarily complicated. I see that for a larger set of unique ranks, a map would be a better choice.

LonMcGregor · 2025-09-10T12:54:23Z

Sprint-3/3-stretch/password-validator.js

+  // Calculate the sum of all digits and check if greater than 16
+  const sum = cardNumber.split('').reduce((acc, digit) => acc + Number(digit), 0);
+  if (sum <= 16) {
+    return false;


Because this is the final step in the code, do you need to use an if(condition) { return false} return true structure, could you simplify this, or is it fine as it is right now?

Hello LonMcGregor,

Thank you for the feedback!

I realized the if statement isn’t necessary here because sum > 16 already evaluates to a boolean. Returning it directly makes the code simpler and easier to read.

HoussamLh and others added 29 commits June 17, 2025 23:30

explain Increment count variable by 1 after initialization.

2dd0a78

Create initials variable by extracting first characters of first, mid…

1bc5c74

…dle, and last names.

create two variable and Extract directory and file extension parts fr…

b16d048

…om file path string using slice.

Add comments explaining how num is calculated as a random integer bet…

625c114

…ween minimum and maximum.

Add explanation for incrementing count variable by 1 after initializa…

f5522e7

…tion.

Update 0.js

2f17199

Added comments to exclude instructional lines from execution.

Update 1.js

5e31d7c

Updated age variable to let to allow incrementing its value

Update 2.js

5cf1cfd

Declare variable before using in template string.

Update 3.js

2d1e8dd

Add comments explaining slice error and fix by converting number to string

implement getAngleType function with all angle cases and tests

4723bb9

implement isProperFraction with tests for positive, negative, equal n…

85a7125

…umerator cases and add stretch scenarios for isProperFraction edge case testing

Implement getCardValue to return correct blackjack values for all val…

a6dd884

…id cards and throw error for invalid ones

Export complete getAngleType() function for Jest testing

cb53f9a

Add Jest tests for all angle types in getAngleType()

5a07227

Implement isProperFraction to handle proper, improper, and negative f…

a417fa0

…ractions

Add unit tests for isProperFraction covering all key scenarios

461d0b6

Implement getCardValue function with support for number, face, and ac…

057740b

…e cards; throw error for invalid cards

Add tests for getCardValue covering number cards, face cards, and inv…

48444ef

…alid input

Revert Sprint-1 folder to CYF's original version

633a7af

Implement countChar function to count occurrences of a character in a…

6d8d7bb

… string

Implement getOrdinalNumber to handle ordinal suffixes correctly

50eaeb4

- Return proper suffixes for 1st, 2nd, 3rd, and others (e.g., 4th, 11th, 23rd) - Added Jest tests for various edge cases including teens (11th-13th) - Improved function to handle general inputs dynamically

Implement repeat function with parameters and error handling

e2c7e37

- Added str and count parameters to repeat function - Used String.repeat to repeat str count times - Throws error if count is negative - Added tests for count = 1, 0, and negative values

Implement find function to locate character index in string with deta…

276c348

…iled comments and explanations

just to writing a missing bits.

28dfefd

Implement credit card validator function

c3ed0d1

- this is just the json files

6d4f165

Merge branch 'coursework/sprint-3' of https://github.com/HoussamLh/Mo…

9a891c6

…dule-Structuring-and-Testing-Data into coursework/sprint-3

recover commits

cbb4e8d

HoussamLh added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Sep 9, 2025

Test: add repeat.test.js to verify repeat function

4c99807

- Added tests for repeat() covering: • repeating a string multiple times • returning empty string when count is 0 • throwing error when count is negative - Cleaned up unused code to follow best practices

LonMcGregor added the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label Sep 10, 2025

LonMcGregor reviewed Sep 10, 2025

View reviewed changes

HoussamLh added 7 commits September 10, 2025 23:08

Resolve the conflict marks from the file

356d4c1

Remove all the blank lines

5040692

Remove the looping over all number cards

f20ecce

simplify number card check with updated regex

f1f5709

expand Case 4 to cover more invalid card inputs

853d408

Improve error message in getCardValue to include invalid card

93ae4e9

Simplify digit sum check by returning boolean expression directly

6fcb856

HoussamLh 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 Sep 12, 2025

HoussamLh mentioned this pull request Sep 12, 2025

London ITP May 25 | Structuring and testing data | Houssam Lahlah HoussamLh/My-Coursework-Planner#50

Open


		if (!rank) throw new Error("Invalid card rank");

		if (rank === "A") return 11;

Uh oh!

London | 25-ITP-May | Houssam Lahlah | Sprint 3 | Coursework/sprint-3 #702

Are you sure you want to change the base?

London | 25-ITP-May | Houssam Lahlah | Sprint 3 | Coursework/sprint-3 #702

Uh oh!

Conversation

HoussamLh commented Sep 9, 2025

Uh oh!

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

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!