Add (German) bank account validation with malkusch/bav

[malkusch]

Hi
I maintain the library malkusch/bav for validation of German bank accounts. This pull requests adds following rules to include bav into your library:

• v::germanBank()
• v::germanBankAccount()
• v::germanBIC()
• v::bank(string $countryCode)
• v::bankAccount(string $countryCode, string $bank)
• v::bic(string $countryCode)

This PR supports only the country code "de".

Enjoy.

[henriquemoody]

@malkusch, first of all, I'm sorry by our late.

This pull request must be rebased with the current master branch, also, I don't think a good idea to have a rule for an specific country, can you do something more generic like what was done in #226?

[malkusch]

German bank validation has more than 100 validation rules. I'm too lazy to extend the validation for other countries. I will rebase if there is demand for a german bank validation.

[henriquemoody]

I didn't say you have to extend the validation for other countries. What I'm trying to avoid is to have germanBank(), braziliamBank(), americamBank() and such... Define a country code in the constructor of the validator sounds too much better for me.
For a while, doesn't matter if you have german* private methods in the validator, but since you keep the API clean we can refactor this when other validators come.

[malkusch]

Ok, Sorry I got you wrong. This sounds of course reasonable. I'll rebase and refactor the validator to a universal API with a country code dependency.

[henriquemoody]

Thanks 😄

[malkusch]

Rebased to master and refactored to Bank($countryCode), BankAccount($countryCode) and BIC($countryCode).

[henriquemoody]

Thanks, @malkusch, now I can review it!

Fist of all, you should fix the detected issues on the Scrutinizer's build.

I will start to review it now.

[henriquemoody]

This is a dependency of de only, so I think you should put it on the same line of de.

[henriquemoody]

If you use Callback instead of AbstractRule as you base class I think it's going to be better, but even using Callback you have to create instances inside the method and it is bad in many aspects because you cannot use dependency injection on it, that's why you need to configure and call malkusch/bav' objects instead of mock them.

I have to think about something better but now my headache doesn't let me do it. 😫

[malkusch]

but even using Callback you have to create instances inside the method and it is bad in many aspects because you cannot use dependency injection on it

Yes, that comes with the universal validator which gets only the country code injected. You probably want to keep the API clean and simple i.e. prefer __construct($countryCode) over __construct(BankLocale $implementation).

[henriquemoody]

I have to think more about it, my first thought was __construct($countryCode, BankCallableChain $bankCallableChain = null) but it doesn't looks like other Respect stuff and doesn't look a final idea. Any thoughts, @alganet?

[henriquemoody]

Any way, it's not a blocker, we can refactor it, since we have a clean API.

@malkusch, just move the validation to the rules' constructors and, if you don't mind, squash these commits (there are too many of them for just 3 rules) and I guess we can merge it as it is.

[malkusch]

Refactored and squashed.

[henriquemoody]

Why not?

$callback = array(new BAV(), 'isValidBIC');

[henriquemoody]

Same here.

[henriquemoody]

Thanks for all your work!!