Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BIC generation is not realistic #1159

Closed
import-brain opened this issue Jul 17, 2022 · 0 comments · Fixed by #1171
Closed

BIC generation is not realistic #1159

import-brain opened this issue Jul 17, 2022 · 0 comments · Fixed by #1171
Labels
c: bug Something isn't working m: finance Something is referring to the finance module s: accepted Accepted feature / Confirmed bug

Comments

@import-brain
Copy link
Member

import-brain commented Jul 17, 2022

While looking at the code mentioned in #1158, I noticed that finance.bic() generates BIC codes in a weird way that might not be representative of real SWIFT/BIC codes. There are two issues:

  1. According to the real SWIFT codes listed on this page (scroll down a bit), it doesn't seem like the 4th character in the code has to be a vowel.

    this.faker.helpers.replaceSymbols('???'),
    this.faker.helpers.arrayElement(vowels),
    However, the function uses helpers.replaceSymbols() to pick three random uppercase letters and then it appends a random vowel to that. To fix this, we could delete line 404 and replace line 403 with this.faker.helpers.replaceSymbols('????').

  2. Also based on the real SWIFT codes mentioned above, it doesn't seem like the 8th char of a SWIFT code is required to be a 1. However, 1 is always the 8th char, because it is hardcoded.

    This char should be randomly generated.

Also, is there any possible way we could make this function easier to read? Personally, the ternaries spread over multiple lines make it incredibly hard to read.

@import-brain import-brain added the c: bug Something isn't working label Jul 17, 2022
@import-brain import-brain added this to the v7 - Current Major milestone Jul 17, 2022
@import-brain import-brain added the s: accepted Accepted feature / Confirmed bug label Jul 22, 2022
@xDivisionByZerox xDivisionByZerox added the m: finance Something is referring to the finance module label Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: bug Something isn't working m: finance Something is referring to the finance module s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants