Skip to content
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

[IMPROVEMENT] Api improvement #35

Closed
HomeOfTheWizard opened this issue May 9, 2023 · 3 comments
Closed

[IMPROVEMENT] Api improvement #35

HomeOfTheWizard opened this issue May 9, 2023 · 3 comments
Labels
enhancement New feature or request
Milestone

Comments

@HomeOfTheWizard
Copy link

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.

@HomeOfTheWizard HomeOfTheWizard added the enhancement New feature or request label May 9, 2023
@henryx
Copy link
Collaborator

henryx commented May 10, 2023

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

@henryx henryx added this to the 6.0.0 milestone May 10, 2023
@HomeOfTheWizard
Copy link
Author

HomeOfTheWizard commented May 12, 2023

great, thanks!
I ve created the following PR #38

@henryx
Copy link
Collaborator

henryx commented May 14, 2023

I close this issue because is merged in mainline

@henryx henryx closed this as completed May 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants