Add structure to allow guessing of BIC based on IBAN #8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Many thanks for your PR. I'll review it soon, but currently I've a lot of work and unfortunately no time. |
| "TF" => %{length: 27, rule: ~r/^[0-9]{10}[0-9A-Z]{11}[0-9]{2}$/i}, # No auto-BIC | ||
| "TG" => %{length: 28, rule: ~r/^[A-Z]{2}[0-9]{22}$/i}, # No auto-BIC | ||
| "TL" => %{length: 23, rule: ~r/^([0-9]{3})[0-9]{16}$/i}, | ||
| "TN" => %{length: 24, rule: ~r/^([0-9]{2})([0-9]{3})[0-9]{13}$/i}, |
There was a problem hiding this comment.
According to the entry at https://de.wikipedia.org/wiki/Internationale_Bankkontonummer the regex should be ~r/^([0-9]{2})([0-9]{3})[0-9]{15}$/i
| "TG" => %{length: 28, rule: ~r/^[A-Z]{2}[0-9]{22}$/i}, # No auto-BIC | ||
| "TL" => %{length: 23, rule: ~r/^([0-9]{3})[0-9]{16}$/i}, | ||
| "TN" => %{length: 24, rule: ~r/^([0-9]{2})([0-9]{3})[0-9]{13}$/i}, | ||
| "TR" => %{length: 26, rule: ~r/^([0-9]{5})0[0-9A-Z]{16}$/i}, |
There was a problem hiding this comment.
The rule should be ~r/^([0-9]{5})[0-9][0-9A-Z]{16}$/i
Comment on lines
+27
to
+32
| case Bankster.Iban.validate(iban) do | ||
| {:ok, canon_iban} -> | ||
| country = Bankster.Iban.country_code(canon_iban) | ||
| bban = Bankster.Iban.bban(canon_iban) | ||
| regexes = Bankster.Iban.iban_rules() | ||
| matches = Regex.run(regexes[country][:rule], bban, [capture: :all_but_first]) |
There was a problem hiding this comment.
What do you think, could with not be "clearer" here?
with {:ok, canon_iban} <- Bankster.Iban.validate(iban),
{:ok, country} <- {:ok, Bankster.Iban.country_code(canon_iban)},
{:ok, bban} <- {:ok, Bankster.Iban.country_code(canon_iban)},
{:ok, regexes} <- {:ok, Bankster.Iban.iban_rules()},
{:ok, matches} <- {:ok, Regex.run(regexes[country][:rule], bban, capture: :all_but_first)} do
calculate_bic(country, matches)
else
{:error, message} ->
{:error, message}
_calculation_failed ->
{:error, :bic_calculation_failed}
end| end | ||
|
|
||
| @spec basic_mapping_nested(%{required(String.t()) => %{required(String.t()) => String.t()}}, String.t(), String.t()) :: {:ok, String.t()} | {:error, :unsupported_bank} | ||
| defp basic_mapping_nested(mapping, bank, branch) do |
There was a problem hiding this comment.
Compiler complains "function basic_mapping_nested/3 is unused". Is this just for demonstration purpouses?
| @@ -309,6 +311,9 @@ defmodule Bankster.Iban do | |||
| @spec valid?(String.t()) :: boolean | |||
| def valid?(iban), do: match?({:ok, _}, validate(iban)) | |||
|
|
|||
| def iban_rules(), do: @iban_rules | |||
There was a problem hiding this comment.
Is this only used as a private function? What is the reason for this function?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a functionality I need for my own project, but I can imagine it would be useful for others as well so I'm submitting it.
As a side note before I explain my changes, this also adds support for Sudan (SD) and Lybia (LY) to the IBAN list.
Basically, I've gone through all the IBAN regexes and added capturing groups to separately capture what can identify the exact bank the IBAN belongs to. In all cases, this is either a single group for just the bank (either directly as a string or as some numerical code) or two groups in which the first is the bank identifier and the second is the branch identifier.
Because there are significantly more banks than there are countries, I've only added proof-of-concept support for The Netherlands (my country), but the approach should be largely similar. After analyzing the IBAN with the associated regex, the captured groups can be passed along to the
calculate_bicmethod. This method should be specifically implemented per country because every country has their own way they denote bank information in their IBAN. Hopefully, users of this library that want to support their own countries can create patches as needed.There's a good amount of IBANs to which I did not yet add capturing groups. These are annotated with
# No auto-BICand are unsupported because I could not figure out what country they belong to or that countries particular IBAN structure.