Repository navigation
London | 26-ITP-Sep | Bartosz Kawiak | Sprint 3 | Stretch Exercises - #1743
bartoszkawiak wants to merge 1 commit into
Conversation
bartoszkawiak
added
📅 Sprint 3
| export function isCardValid(cardNum) { | ||
| if (typeof cardNum !== "number") { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Because of the way JavaScript represents numbers, isCardValid(9999999999999997) would evaluate to true.
Could you use AI to find out why, and then update your code (both function implementation and tests) to use other data type to represent card number?
| const regex = /^\d$/; | ||
| if (!regex.test(num)) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Note: Since you are using regex, you could also just use regex to check if the whole card number (as a string) contain 16 digits (outside the loop) instead of checking each digit one by one.
| test("card number should have 16 digits", () => { | ||
| const cardNumber = 1234567890987654; | ||
| const result = isCardValid(cardNumber); | ||
| expect(result).toEqual(true); | ||
| }); |
There was a problem hiding this comment.
In addition to checking if the function can correctly return true when the specified condition is met, this test should also check if the function can correctly return false when the specified condition is not met.
The same also applies to other tests in this script.
| hasSpecialChars && | ||
| !passwords.includes(password) | ||
| ) { | ||
| passwords.push(password); |
There was a problem hiding this comment.
Is "adding the password being checked to a global variable" an expected behavior of this function?
| const passwords = []; | ||
| export function isValidPassword(password) { |
There was a problem hiding this comment.
For higher flexibility, we could design the function to accept passwords as an optional second parameter (with a default value).
| const specialChars = "!#$%.*&"; | ||
|
|
||
| for (const char of password) { | ||
| hasUpper = upperLetters.includes(char) ? true : hasUpper; |
There was a problem hiding this comment.
Note:
hasUpper = upperLetters.includes(char) ? true : hasUpper;is equivalent to
hasUpper = hasUpper && upperLetters.includes(char);or
if (upperLetters.includes(char)) hasUpper = true;Alternatively, you could use regex to check if password contains an uppercase letter in one statement.
| for (const char of password) { | ||
| hasUpper = upperLetters.includes(char) ? true : hasUpper; | ||
| hasLower = lowerLetters.includes(char) ? true : hasLower; | ||
| hasNumbers = numbers.includes(char) ? true : hasNumbers; |
There was a problem hiding this comment.
Note: The term digit refers to a number in the range 0-9. A number may contain any number of digits.
| test("password have at least one number(0-9)", () => { | ||
| const password = "RRRqqq"; | ||
| const result = isValidPassword(password); | ||
| expect(result).toEqual(false); | ||
| }); |
There was a problem hiding this comment.
The function can return false for multiple reasons.
To test a specific reason, choose an input that satisfies all other conditions except the one you're targeting. This way, if the function returns false, you can confidently attribute it to that specific condition.
For example, the function might return false for "RRRqqq" not only because it lacks a digit, but also because it lacks a special symbol. As a result, we can't be certain that the function correctly handles the case of passwords have at least one digit, since multiple conditions are being violated simultaneously.
cjyuan
added
Reviewed
Learners, PR Template
Self checklist
Task code
CYF-1065
Changelist
Stretch exercises solved, implemented tests and finished function. Added and finished required files for card-validation.
Questions
I did change the password in first example test in password-validation.test.js as I did not find a way to make it pass alongside other requirements. Is there a way to complete the exercise with the example test passing?