fix(iban): more strict pattern for IE and PS#3464
Conversation
✅ Deploy Preview for fakerjs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #3464 +/- ##
=======================================
Coverage 99.97% 99.97%
=======================================
Files 2819 2819
Lines 217500 217500
Branches 953 952 -1
=======================================
Hits 217447 217447
Misses 53 53
🚀 New features to boost your workflow:
|
Shinigami92
left a comment
There was a problem hiding this comment.
This PR changes the used algorithm from type c to a which then does not generate digits anymore
faker/src/modules/finance/index.ts
Lines 921 to 929 in 048c325
I guess you answered that because the implementation wasn't that clear? At least, that's what I was thinking when starting to work on the issue. I'm certain we could update this code (in a separate PR) to make it more readable. I'm also not sure if we need all featured that the Iban data structure provides 🤷 |
Yes, all good |
I agree the IBAN code is quite incomprehensible. Even some minor changes like defining a constant for "c" and "a" at top of the file would help readability |
|
I agree with both of you. I'll work on a second PR to make this more readable. I'm just wondering whether we should consider performance metrics here. While I could immediately think of multiple ways of how the implementation could be written more readable, all would definitely be slower (mostly due to multiple iterations over the expected pattern). |
I'm also most times in favor of readability and maintainability over performance. JS is not the language of choice when it comes to processing millions of data efficiently anyway. |
Description
This PR fixes #3463.
How to Test
To verify the change you can open the deploy review of the docs and past the following in the dev console:
The expected output is the log that tells you that all ibans were valid. If you want to repeat the test (with newly generated ibans) you can simply call
testRandomIban()in the dev console again.