search

jopenlibs / vault-java-driver

Zero-dependency Java client for HashiCorp's Vault
https://jopenlibs.github.io/vault-java-driver
26 stars 18 forks source link

[IMPROVEMENT] Api improvement #35

Closed HomeOfTheWizard closed 1 year ago

HomeOfTheWizard commented 1 year ago

Hi,

First of all, Kudos for the initiative to continue evolving the vault-java-driver library! 👍 🙏

I would like to suggest a change in the API. I think a Public Interface instead of a Class would be better for the clients, and the maintainers of the API. This pattern has several advantages, as explained in effective Java - Item 1.

For myself, I am using the library from betterclouds, and find it not easy to mock and test. Also it would be easier for the client to decorate the class if an interface was available, for example if I want to add a wrapper class for caching. For the maintainers, it would allow to change the implementation easily without changing the public API.

Since there is no other project using your project, as I can see from github, It would be easy for you to change the API and hide the implementation class. Or you can keep it, in case you want to maintain retrocompatibility with clients using the old library from betterclouds.

I did a fork, if you are interested in the change I can do a PR so you can review.

henryx commented 1 year ago

Hi,

I think is a good idea. It can be integrated in next major release. Feel free to make a PR, I hope to merge it as soon as possible

HomeOfTheWizard commented 1 year ago

great, thanks! I ve created the following PR https://github.com/jopenlibs/vault-java-driver/pull/38

henryx commented 1 year ago

I close this issue because is merged in mainline