-
Notifications
You must be signed in to change notification settings - Fork 127
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.
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
Add support for extension points #87
base: master
Are you sure you want to change the base?
Conversation
…h SPI (ServiceLoader)
…h SPI (ServiceLoader)
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. 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. |
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, |
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. and the loader: Now, I have a 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, |
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
BbanStructure
s can be provided.This, now, can be done by providing an implementation of
BbanStructureProvider
, and add the fully qualified name of your implementation insrc/main/resources/META-INF/services/org.iban4j.spi.BbanStructureProvider
of your project.The project currently have one implementation, that is
DefaultBbanStructureProvider
, which holds all theBbanStructure
s that were there before this change, and a test implementation of Iraq.