byrokrat / banking

Data types and factories for bank accounts in the swedish banking system, Handelsbanken, ICA-banken, Nordea, SEB, Skandiabanken, Swedbank, PlusGirot, Bankgirot among others.
The Unlicense
11 stars 10 forks source link

Don't report as unknown if cleringnummer matches #13

Closed puggan closed 6 years ago

puggan commented 6 years ago

If i have the right cleringnumer, but the wrong lenth, I get no errors, I just get an Unknown bank.

One way to solve that would be to report all validation errors, if the clearingnumber match.

Works in my cases, hope it works for all cases.

puggan commented 6 years ago

PHP Fatal error: Class 'PHPUnit_Framework_TestCase' not found

Looks like composer.json in master is missing the phpunit/phpunit

puggan commented 6 years ago

Odd error: "could not create session key: disk quota exceeded" https://travis-ci.org/byrokrat/banking/jobs/300187932

hanneskod commented 6 years ago

Hi and thanks for this pr! I think your suggestion is valid. The reason it’s currently not implemented is that the official registry of clearing numbers is being continuously updated. And as a defensive coder I was afraid that parsing valid account numbers could break whit an out of date internal list of account formats.

Maybe that’s to defensive, we can assume that bgc does not reuse clearing numbers lightly. I would still however like to keep a defensive flair and make this behavior opt-in in some way. Not sure how the interface should look like though. Adding more arguments to AccountFactory::construct() feels like a no-go as it is way too complex already. Maybe add something like an AccountFactoryBuilder..

Need to think some more on this...

hanneskod commented 6 years ago

Hi and sorry for the delay. I wanted to fix this earlier, but I found your implementation a little to complex for me. While working on it I found a few other things that needed fixing too, so in the end I did a rewrite.

Please have a look at the 2.0 branch.