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

Add BankAccountNumber property to Iban class #203

Open
IanKemp opened this issue Jul 2, 2024 · 4 comments
Open

Add BankAccountNumber property to Iban class #203

IanKemp opened this issue Jul 2, 2024 · 4 comments

Comments

@IanKemp
Copy link

IanKemp commented Jul 2, 2024

Is your feature request related to a problem? Please describe.
IbanBuilder has a WithBankAccountNumber(string) method, yet there is no corresponding BankAccountNumber property on the Iban object. This means it's possible to build up an IBAN from its various parts, but impossible to get those same parts back out without performing manual parsing, which is a confusing API experience.

Describe the solution you'd like
Add property string BankAccountNumber on Iban.

Describe alternatives you've considered
Parsing.

Additional context
N/A

@skwasjer
Copy link
Owner

skwasjer commented Jul 3, 2024

The local bank account number (not BBAN) itself cannot easily be isolated atm. as this is not defined in the specification (because this is different per country and sometimes even per bank). It isn't just the case that one can subtract the bank ID and branch ID (as far as I am aware) from the BBAN to give you the local bank account number.

This API experience is maybe not intuitive/consistent, I kindof agree, but adding a BankAccountNumber to the Iban property as proposed is not feasible because of this. If you really need the local bank account number without the bank/branch ID, I see no other way than that custom extract logic is needed. It could potentially be added, but only if there is a specification somewhere that could be referred to.

The Iban type does expose the Bban which can be used in the builder though. The builder is a bit more 'relaxed' in terms of the input for WithBankAccountNumber, as it accepts the full BBAN but also a local bank account number that does not come with the bank ID and/or branch ID. These can be (optionally) supplied separately via the WithBankIdentifier/WithBranchIdentifier builder methods.

So this would work:

var parser = new IbanParser(IbanRegistry.Default);
var inputIban = "GB29NWBK60161331926819";
Iban parsedIban = parser.Parse(inputIban);

string builtIban = new IbanBuilder()
    .WithCountry("GB", IbanRegistry.Default)
    .WithBankAccountNumber(parsedIban.Bban)
    .WithBankIdentifier(parsedIban.BankIdentifier)     // Technically redundant, as BBAN already has bank ID.
    .WithBranchIdentifier(parsedIban.BranchIdentifier) // Technically redundant, as BBAN already has branch ID.
    .Build();

Console.WriteLine(builtIban == inputIban);

To further illustrate, these examples should all yield identical results:

// Only with BBAN
string iban1 = new IbanBuilder()
    .WithCountry("GB", registry)
    .WithBankAccountNumber("NWBK60161331926819")
    .Build(); // GB29NWBK60161331926819

// Branch + account nr are combined, and bank provided separately.
string iban2 = new IbanBuilder()
    .WithCountry("GB", registry)
    .WithBankAccountNumber("60161331926819")
    .WithBankIdentifier("NWBK")
    .Build(); // GB29NWBK60161331926819

// All parts separate.
string iban3 = new IbanBuilder()
    .WithCountry("GB", registry)
    .WithBankAccountNumber("31926819")
    .WithBankIdentifier("NWBK")
    .WithBranchIdentifier("601613")
    .Build(); // GB29NWBK60161331926819

// Or, you can even 'overwrite' bank/branch of the bank account number:
string iban4 = new IbanBuilder()
    .WithCountry("GB", registry)
    .WithBankAccountNumber("ABCD000000331926819") // Full BBAN but with different bank/branch.
    .WithBankIdentifier("NWBK")
    .WithBranchIdentifier("601613")
    .Build(); // GB29NWBK60161331926819

The order in which you call the builder methods does not matter, the builder always does this:

  1. write bank account number to buffer
  2. overwrite bank ID (if provided)
  3. overwrite branch ID (if provided)
  4. compute check digits

I understand this does not solve your problem. But in relation to your OP, we can improve the documentation of course or perhaps you have other suggestion/idea?

@IanKemp
Copy link
Author

IanKemp commented Jul 19, 2024

The local bank account number (not BBAN) itself cannot easily be isolated atm. as this is not defined in the specification (because this is different per country and sometimes even per bank)

What exactly are the various Pattern classes then, if not an implementation of the SWIFT specification? E.g. Patterns.GB is defined as SwiftPattern("GB2!n4!a6!n8!n", 22, true) which translates to: literal GB, 2 check digits, 4 ASCII characters (BIC), 6 numeric characters (bank code), 8 numeric characters (account number) - which is completely correct for the UK.

Given that you already use those patterns for (presumably) building an IBAN, what prevents you from also using them to parse an IBAN into its constituent pieces? In cases where a specific country doesn't have the concept of "account number", the relevant property on the Iban class could simply return null or throw some sort of exception.

@skwasjer
Copy link
Owner

The patterns and information taken from SWIFT only provide so much information, and are clearly intended for validation of IBAN, but says nothing really clearly about local account numbers. It does provide where the BBAN, branch ID and bank ID are located, but does not explicitly state what is considered to be the local account number. Perhaps, in certain countries, the bank ID is part of the local account number? I don't know and can't really tell. Maybe I am overcautious...

I've made exactly this assumption in the past that: for every country, the local account number is simply the BBAN without bank ID and/or branch ID. But since it was an assumption, I did not feel comfortable adding it. Or maybe I am just not interpreting the spec correctly, idk. And noone ever really asked until now 😉

I am willing to add this. If devs/apps can start using it, I will find out sooner or later 😄

@IanKemp
Copy link
Author

IanKemp commented Jul 22, 2024

The patterns and information taken from SWIFT only provide so much information, and are clearly intended for validation of IBAN, but says nothing really clearly about local account numbers. It does provide where the BBAN, branch ID and bank ID are located, but does not explicitly state what is considered to be the local account number. Perhaps, in certain countries, the bank ID is part of the local account number? I don't know and can't really tell. Maybe I am overcautious...

I've made exactly this assumption in the past that: for every country, the local account number is simply the BBAN without bank ID and/or branch ID. But since it was an assumption, I did not feel comfortable adding it. Or maybe I am just not interpreting the spec correctly, idk. And noone ever really asked until now 😉

I wouldn't call it an assumption, more a case of a de facto standard versus a codified one. Even so, it's a pretty reliable standard IMO because the likelihood of a country arbitrarily changing how their IBANs are constructed after the fact would appear to be rather low, given how much software is already making the same "assumptions" about the format of those IBANs.

I am willing to add this. If devs/apps can start using it, I will find out sooner or later 😄

That would be greatly appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants