skwasjer / IbanNet

C# .NET IBAN validator, parser, builder and generator
Apache License 2.0
126 stars 31 forks source link

Add `BankAccountNumber` property to `Iban` class #203

Open IanKemp opened 3 months ago

IanKemp commented 3 months ago

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 commented 3 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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.