LONDON_JAN25 | KHALIL ALI | STRUCTURING_AND_TESTING_DATA | SPRINT 3 | WEEK 6 #388

khalil-ali1 · 2025-03-13T10:37:39Z

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

Briefly explain your PR.

Questions

Ask any questions you have for your reviewer.

cjyuan · 2025-03-24T17:54:05Z

Please follow the "Code Review Process" described here and add the appropriate label to your PR. Otherwise the volunteers will not know this PR needs a review.

cjyuan

Most of the code is good.

There are a few bugs, and some of the code can use some improvement.

cjyuan · 2025-04-02T16:24:15Z

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

+    if (denominator === 0) return "Error: Denominator cannot be 0";
+    if (Math.abs(numerator)  < denominator) return true;
+    if (Math.abs(numerator) >= denominator) return false;


We can also design the function to return false when the denominator is zero. Technically, anything divided by zero is not a proper fraction.

Denominator can also be negative. Mathematically speaking, 2/-3 and -2/-3 are proper fractions.

For any value that is not a proper fraction, we can just return false without an if-statement.

you are right,
now it is more concise and readable.

cjyuan · 2025-04-02T16:25:19Z

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

+test("should identify reflex angle (> 180° but < 360°)", () => {
+  expect(getAngleType(270)).toEqual("Reflex angle");
+});


We can specify multiple expect(...) statements within each test() to cover multiple values that belong to the same case. For example,

test("should identify reflex angle (> 180° but < 360°)", () => { expect(getAngleType(300)).toEqual("Reflex angle"); expect(getAngleType(359.999)).toEqual("Reflex angle"); expect(getAngleType(180.001)).toEqual("Reflex angle"); });

thank you for your comment.
I added more examples to make a more comprehensive test.

cjyuan · 2025-04-02T16:27:09Z

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

+test('should throw "Invalid card rank" for X♠', () => {
+    expect(() => getCardValue("X♠")).toThrow("Invalid card rank");
+});


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

getCardValue("010♠"); getCardValue("02♠"); getCardValue("0x02♠"); getCardValue("2.1♠")

I fixed the bug.
now it returns like expected.

cjyuan · 2025-04-02T16:29:42Z

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

@@ -22,3 +22,9 @@ test("should count multiple occurrences of a character", () => {
 // And a character char that does not exist within the case-sensitive str,
 // When the function is called with these inputs,
 // Then it should return 0, indicating that no occurrences of the char were found in the case-sensitive str.
+test("should count multiple occurrences of a character", () => {


This description does not seem to match the test specified.

I fixed the description

cjyuan · 2025-04-02T16:30:43Z

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

@@ -1,5 +1,18 @@
 function getOrdinalNumber(num) {
- return "1st";
-}
+    if (num === 11 || num === 12 || num === 13) {


The ordinal numbers of 111, 312, 9913 are "111th", "312th", "9913th".

I fixed the bug.
I added more examples to the test function.

cjyuan · 2025-04-02T22:04:07Z

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

@@ -0,0 +1,37 @@
+const isValidNumber = (cardNum) => {
+    const StrCardNum = cardNum.toString();


Common convention is to start a variable name with a lowercase letter. Uppercase camel case is usually applied to Class or Type names.

I changed it.

cjyuan · 2025-04-02T22:12:40Z

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

+    if (cardNum % 2 !== 0) {
+        return false;
+    }


This approach is not safe. For example, 9999888877776665 % 2 == 0 is true.

Can you replace this check so that it will work on any 16 digit credit card number (regardless if the number is represented as a string or a value of type number)?

to check the last digit whether even or odd

cjyuan · 2025-04-02T22:24:37Z

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

+});
+
+test('Should return false if the sum < 16', () => {
+    expect(isValidNumber(1011111111111111)).toBe(false);


Can you try also expect(isValidNumber(1111111111111110)).toBe(false);? It can reveal a bug in your code.

Also, 1111111111111110 is a more suitable test value than 1011111111111111 in this test case. Can you figure out why?

I fixed the bug.
I think because 1111111111111110 is even and the sum of all digits is<16.

cjyuan · 2025-04-02T22:29:03Z

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

@@ -0,0 +1,37 @@
+const isValidNumber = (cardNum) => {


Because of the limited precision of number type in JavaScript, I would assume cardNum is of type string.

I fixed it.

cjyuan · 2025-04-02T22:33:08Z

Sprint-3/4-stretch-investigate/password-validator.test.js

+test("valid passwords", () => {
+    expect (isValidPassword("1Aa2345!")).toEqual(true); 
+})
+test("valid passwords", () => {
+    expect (isValidPassword("1Aa2345")).toEqual(false); 
+})


We should also include tests to check if the function can return false when the password fails only one of the following conditions:

Have at least 5 characters.

Have at least one English uppercase letter (A-Z)

Have at least one English lowercase letter (a-z)

Have at least one number (0-9)

Have at least one of the following non-alphanumeric symbols: ("!", "#", "$", "%", ".", "*", "&")

Must not be any previous password in the passwords array.

I added 6 testing lines. Each one breaks only one condition.

I added more testing examples.

add more testing examples.

khalil-ali1 added 13 commits February 22, 2025 18:06

finish 1-key-implement/1-get-angle-type

81e3b58

finish 1-key-implement/2-is-proper-fraction

b0d6861

finish 1-key-implement/3-get-card-value

d554fe0

finish 2-mandatory-rewrite/1-get-angle-type

4b83ace

finish 2-mandatory-rewrite/2-is-proper-fraction

619125a

finish 2-mandatory-rewrite/3-get-card-value

7452251

finish 3-mandatory-practice/count

f770750

finish 3-mandatory-practice/get-ordinal-number

2fdc4e3

finish 3-mandatory-practice/repeat

a67b5a0

fix test line place

4ea450e

finish 4-stretch-investigate/password-validator

44a38f4

finish 4-stretch-investigate/card-validator

df074c1

add more tests

9360469

khalil-ali1 added the Needs Review Participant to add when requesting review label Apr 2, 2025

cjyuan added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Participant to add when requesting review labels Apr 2, 2025

cjyuan reviewed Apr 2, 2025

View reviewed changes

cjyuan added Reviewed Volunteer to add when completing a review and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Apr 2, 2025

khalil-ali1 added 7 commits April 3, 2025 09:32

improve isProperFraction function

2dd6e9c

add more examples to the test function

8c20644

fix 2-9 cards bug

d1baa4b

changed the test function description

d6da377

I fixed the bug.

8579bc7

I added more testing examples.

response to all comments/add more examples for the test function

5d66d44

fix the function.

fa01d56

add more testing examples.

khalil-ali1 added the Complete Participant to add when work is complete and review comments have been addressed label Apr 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LONDON_JAN25 | KHALIL ALI | STRUCTURING_AND_TESTING_DATA | SPRINT 3 | WEEK 6 #388

LONDON_JAN25 | KHALIL ALI | STRUCTURING_AND_TESTING_DATA | SPRINT 3 | WEEK 6 #388

khalil-ali1 commented Mar 13, 2025 •

edited

Loading

cjyuan commented Mar 24, 2025

cjyuan left a comment

cjyuan Apr 2, 2025

khalil-ali1 Apr 3, 2025

cjyuan Apr 2, 2025

khalil-ali1 Apr 3, 2025

cjyuan Apr 2, 2025

khalil-ali1 Apr 3, 2025

cjyuan Apr 2, 2025

khalil-ali1 Apr 3, 2025

cjyuan Apr 2, 2025

khalil-ali1 Apr 4, 2025

cjyuan Apr 2, 2025

khalil-ali1 Apr 4, 2025

cjyuan Apr 2, 2025

khalil-ali1 Apr 4, 2025

cjyuan Apr 2, 2025

khalil-ali1 Apr 4, 2025

cjyuan Apr 2, 2025

khalil-ali1 Apr 4, 2025

cjyuan Apr 2, 2025

khalil-ali1 Apr 4, 2025

		@@ -0,0 +1,37 @@
		const isValidNumber = (cardNum) => {
		const StrCardNum = cardNum.toString();

LONDON_JAN25 | KHALIL ALI | STRUCTURING_AND_TESTING_DATA | SPRINT 3 | WEEK 6 #388

Are you sure you want to change the base?

LONDON_JAN25 | KHALIL ALI | STRUCTURING_AND_TESTING_DATA | SPRINT 3 | WEEK 6 #388

Conversation

khalil-ali1 commented Mar 13, 2025 • edited Loading

Learners, PR Template

Changelist

Questions

cjyuan commented Mar 24, 2025

cjyuan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

khalil-ali1 commented Mar 13, 2025 •

edited

Loading