fix(utils): correct bank number validation#917
Conversation
mmoehabb
left a comment
mmoehabb
left a comment
There was a problem hiding this comment.
Please write unit tests for this method.
DevMuhammed3
force-pushed
the
devmo/valid-bank-number
branch
from
c1fe6b0 to
059b72c
Compare
| .reduce((sum, digit) => sum + digit, 0) % | ||
| 10 === | ||
| 0; |
| .reverse() | ||
| .map((char, index) => { | ||
| const digit = Number(char); | ||
| return index % 2 === 1 |
There was a problem hiding this comment.
Prefer writing declarative code; use isEven or isOdd functions.
| if (isNaN(Number(bankNumber))) { | ||
| return FieldError.InvalidBankAccountNumber; | ||
| } | ||
|
|
There was a problem hiding this comment.
The length of bank numbers varies between 9 digits to 12. We may use that as well before commencing on the luhn algorithm.
| ? digit * 2 > 9 | ||
| ? digit * 2 - 9 | ||
| : digit * 2 | ||
| : digit; |
There was a problem hiding this comment.
This is unreadable at all. It's logic is correct 100%, but it's hard to read. Try using if statements instead.
Moreover, try to write the algorithm yourself first:
Algorithm isValidBankAccount:
Input: bankNumber - string of the bank number
Output: true or false
sum <- 0
for each digit in the bank number do the following
d <- the digit
if the digit is an even number
sum <- sum + d
continue
...
Remember, you will never learn by allotting everything to the AI.
You way manna write the function in an empty JS file in order to be able to experiment and execute it easily with node.
| import { | ||
| generateValidLuhnNumber, | ||
| generateInvalidLuhnNumber, | ||
| } from "./luhnGenerator"; |
There was a problem hiding this comment.
Relative imports are not recommended.
There was a problem hiding this comment.
This's not how we write unit tests. Use AI as a tool only. You are the coder; read other unit tests files and follow its pattern or so to speak our implicit convention of writing tests.
the unit tests should be added in the
invoice.test.tsfile.
There was a problem hiding this comment.
Thank you for the note🫡
There was a problem hiding this comment.
First of all, only test files, like {entity}.test.ts, are stored in the tests directory. This shouldn't be store here. You may write these functions within the file that uses them. Second, I don't believe this is the best approach. I prefer using just multiple valid and invalid bank account numbers.
We may use the ones being used by check50:
https://github.com/cs50/problems/blob/2025/x/credit/__init__.py
3 participants
Quality Checklist
Links
Important
Incase you have any valuable knowledge that you think that it is worth writing down, feel free to add it to our handbook. Wounder why we are doing this? check this guide.