arturmkrtchyan / iban4j

A Java library for generation and validation of the International Bank Account Numbers (IBAN ISO_13616) and Business Identifier Codes (BIC ISO_9362).
http://iban4j.org
Apache License 2.0
271 stars 124 forks source link

Add support for extension points #87

Open albertlr opened 2 years ago

albertlr commented 2 years ago

The current implementation is not flexible enough. If a given country was not implemented or not supported at the time of writing, without a code change there is no way to add support for a new country that now do support IBAN accounts.

In my example I used Iraq, which is not implemented in the code, and cannot be extended, only with a code change and a new release, which might be cumbersome for the lifecycle of the project that you are working on.

So I added an extension point through SPI, where additional BbanStructures can be provided.

This, now, can be done by providing an implementation of BbanStructureProvider, and add the fully qualified name of your implementation in src/main/resources/META-INF/services/org.iban4j.spi.BbanStructureProvider of your project.

The project currently have one implementation, that is DefaultBbanStructureProvider, which holds all the BbanStructures that were there before this change, and a test implementation of Iraq.

FrankHossfeld commented 2 years ago

In my opinion, a not implemented country is a bug. Therefore it needs an issue and a contributor to fix it.

Building a service loader adds a cool feature, that will help you to shorten the time until the bug is fixed and your application does no longer depend on a new release, but it does not solve the problem. It is just nothing else as a workaround for a different problem: the release cycles of this lib are too slow.

And - many users of this lib will add always the same missing informations. Personally, I don't like that.

I cloned the repo and created my own implementation. Not, because of the slow release cycles. I needed the lib in a GWT/J2CL-implementation. But there were some problems in the implementation, I had to fix. Now, I can use the lib in a GWT-, J2CL- and Java implementation and besides that, have not to wait for an update of this lib in case of a bug. (btw, the API is the same) On the other hand, I have to do the work by myself.

I adapted your idea and changed some things to get rid of the static methods.

Btw, you should also add an issue for the missing Iraq structure.

albertlr commented 2 years ago

Hi @FrankHossfeld ,

Thanks for your feedback. Actually our problem wasn't with Iraq, as we don't do business with them, however to prove my point, that one was also missing.

Our main concern is that the current implementation does not support all countries that adopted IBAN, and some were updated (or were wrong right from the beginning - eg Seychelles). In our particular case it would help with Seychelles by providing our own implementation, and still allowing us to work with the library as is. I would be also beneficial for us in our testing where we could also provide fictive codes where other systems should react.

The first thing what we try to avoid when using an open-source project is to avoid "maintaining" our own version of the library. We would rather contribute.

Regards, Róbert

FrankHossfeld commented 2 years ago

Hi Róbert,

Ok, that was not clear. Thought you were missing the IBAN definition of Iraq. As I checked my implementation and saw, that Sudan and Libyan is also missing, I think.

I'll adopted your idea and use a BBAN structure provider with a separat loader. The loader uses a method inside the provider to add a BBAN structure. In my use case I could not use something that is based on META-INF.

See: https://github.com/NaluKit/iban4g/blob/main/iban4g/src/main/java/com/github/nalukit/iban4g/shared/bban/BbanStructureProvider.java

and the loader:

See: https://github.com/NaluKit/iban4g/blob/main/iban4g/src/main/java/com/github/nalukit/iban4g/shared/bban/loader/DefaultBbanStructureProviderLoader.java

Now, I have a addBbanStructure-method inside the provider to load missing BBAN structures. All tests are running and it looks like there are no difference in performance.

If you like, you can give it a try. API should be the same. At least, you need only switch the dependency and change the package name. I think, that's it. If you have any questions, feel free to contact me: https://gitter.im/Nalukit42/Lobby

Regards, Frank