London | 26-ITP-May | Tomislav Dukez | Sprint 3 | Stretch#1454
Conversation
This comment has been minimized.
This comment has been minimized.
tomdu3
added
🏝️ Priority Stretch
tomdu3
changed the title
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
removed
the
Needs Review
tomdu3
added
📅 Sprint 3
| // Check if the card number is a positive integer. | ||
| if (typeof cardNumber !== "number" || cardNumber < 0) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Note:
-
To check if a value is in integer,
Number.isInteger()is better. -
Credit card numbers typically have 16 digits, but not all 16-digit integers can be safely represented as a number in JS -- many numbers larger than
Number.MAX_SAFE_INTEGERcannot be represented properly. For example,9007199254740993. Normally, card numbers are represented as strings.
No change required.
| // In case of the call find("code your future", "z"), the index variable updates as follows: | ||
| // 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14 - when it stops, because the condition index < str.length is no longer true | ||
| // the and because the loop has arrived to the end of the string without finding "z". |
There was a problem hiding this comment.
In case of the call
find("code your future", "z"), the index variable updates as follows:
The last value of index in this function call is not 14.
| - Have at least one of the following non-alphanumeric symbols: ("!", "#", "$", "%", ".", "*", "&") | ||
| - Must not be any previous password in the passwords array. | ||
| */ | ||
| const passwords = ["pa$$w0rd", "Qwerty1#", "Adm1n2#", "$3cr4t"]; |
There was a problem hiding this comment.
Could consider design the function to accept the "used passwords" through an optional parameter with default value.
There was a problem hiding this comment.
Thanks. That actually gives a more elegant solution.
| !/[A-Z]/.test(password) || | ||
| !/[a-z]/.test(password) || | ||
| !/[0-9]/.test(password) || | ||
| !/[!#\$%\.\*&]/.test(password) || |
There was a problem hiding this comment.
Some of the characters in the [...] on line 16 do not need to be escaped.
Could you look this up?
Inside
[...]of a regular expression in JS, which characters need to be escaped?
There was a problem hiding this comment.
Thank you CJ. I am only confused with the regex characters here. I was convinced $, . and * needed to be escaped, because otherwise they would considered as special regex chars. But thanks.
| test("password has at least one English lowercase letter (a-z)", () => { | ||
| // Arrange | ||
| const password = "1234Aa%"; | ||
| // Act | ||
| const result = isValidPassword(password); | ||
| // Assert | ||
| expect(result).toEqual(true); | ||
| }); |
There was a problem hiding this comment.
Note:
-
One check for valid case is probably enough. Besides, having a lowercase letter (or digit, etc.) does not necessary make a password valid.
-
Your tests for invalid cases are comprehensive. Well done.
cjyuan
added
Reviewed
2 participants
Learners, PR Template
Self checklist
Changelist
Added tasks:
findpassword validatorcard validator