Skip to content

Validation of IBAN (Internation Bank Account Number) #629

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 6 commits into from
Closed

Validation of IBAN (Internation Bank Account Number) #629

wants to merge 6 commits into from

Conversation

digitalica
Copy link
Contributor

Please have a look at this. Although it works for me, there are a few things to think about:

  • IBAN will be introduced all over EU (and more) over the next years, maybe it should be in the main validate.js, and not in additional methods? At least it is a reason to think it over carefully: naming, location, maybe split per country.
  • the code is quite long, maybe it can be shorter / more efficient? Especially the way the regexp for the country specific check can be build in other ways, or even be defined directly for every country (longer, but more clear: now a 'definition' string is parsed to build the regexp)
  • There is a country-specific format check done now, just before the 'real' checksum check. This helps to block invalid IBANs (correct checksum, but incorrect country specific format) but will also block valid IBANS if a new country starts to use IBAN or if a country specific format will change in the future (I doubt the latter will ever happen, the first will though). So I think we should eigther provide 2 checks (iban and ibanwithoutcountrycheck) OR allow unknown countries, and just for those not check the format....
  • At least in theory the check could be more specific, as within the 'local' account number there can be another checksum / limitations that are not checked now. I think this is not really needed though, as the checksum in the IBAN itself is quite strong and countries may stop using their local restrictions anyway to just rely in IBAN in the future.

@nschonni
Copy link
Collaborator

nschonni commented Feb 6, 2013

This looks alot like your other PR #628 because I think you branched of it to create this one. You might try

  • git checkout upstream/master
  • git checkout -b cleaned-iban-validation
  • git cherry-pick afefdf4
    Then push an resubmit your PR.

@digitalica
Copy link
Contributor Author

ok, i'll give it a try. (and close this)

@digitalica digitalica closed this Feb 6, 2013
@digitalica
Copy link
Contributor Author

That moved the IBAN stuff in the last PR... making things even worse ;-)

Robbert

On Wed, Feb 6, 2013 at 8:59 PM, Nick Schonning notifications@github.comwrote:

It looks like you're other pull #628https://github.com/jzaefferer/jquery-validation/issues/628because you last PR was working of master. You might try

Robbert Wethmar
Digitalica
Jan Evertsenstraat 144 A4
1056 EK Amsterdam
phone 06 53804879

@digitalica
Copy link
Contributor Author

Ok, have retried now, and PR #630 looks better, although I expect it to
break when the other one is merged, but if it does it should be a very
simple to fix.

On Wed, Feb 6, 2013 at 9:17 PM, Robbert Wethmar robbert@digitalica.nlwrote:

That moved the IBAN stuff in the last PR... making things even worse ;-)

Robbert

On Wed, Feb 6, 2013 at 8:59 PM, Nick Schonning notifications@github.comwrote:

It looks like you're other pull #628https://github.com/jzaefferer/jquery-validation/issues/628because you last PR was working of master. You might try

Robbert Wethmar
Digitalica
Jan Evertsenstraat 144 A4
1056 EK Amsterdam
phone 06 53804879

Robbert Wethmar
Digitalica
Jan Evertsenstraat 144 A4
1056 EK Amsterdam
phone 06 53804879

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@digitalica @nschonni