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

IPaymentClient should inherit from IDisposable #319

Closed
woutware opened this issue Aug 30, 2023 · 7 comments
Closed

IPaymentClient should inherit from IDisposable #319

woutware opened this issue Aug 30, 2023 · 7 comments

Comments

@woutware
Copy link

woutware commented Aug 30, 2023

As some IPaymentClient implementations internally use HttpClient objects, which are disposable, the IPaymentClient itself should also be disposable. So ideally the IPaymentClient interface inherits from IDisposable. If not all implementations are disposable, you could make individual implementations implement IDisposable, but I think more user friendly would be to let the interface inherit it.

Also you might need to check if the HttpClient was passed to the payment client from the outside, in which case the payment client is not the owner of the http client, and therefore should not dispose it. But if the HttpClient was created by the payment client, it should dispose of it.

@Viincenttt
Copy link
Owner

Hi @woutware ,

Good idea, I agree with your suggestion. The recommended way to use the Mollie .NET client is to use the recently introduced dependency injection extension method, as described in the documentation:
https://github.com/Viincenttt/MollieApi#dependency-injection

In case someone is not using dependency injection and is using the constructor variant where no HttpClient instance is provided, I'll see if I can add some code to dispose the HttpClient instance that is created by the Mollie API classes.

Kind regards,
Vincent

Viincenttt pushed a commit that referenced this issue Sep 3, 2023
Viincenttt added a commit that referenced this issue Sep 3, 2023
* #319 - BaseMollieClient now implements IDisposable to dispose HttpClient

* #319 - All client interfaces now inherit from IDisposable

* #319 - All integration tests now dispose of API clients

* #319 - Update documentation on IDisposable
@Viincenttt
Copy link
Owner

This is now done and will be released in the next version

@woutware
Copy link
Author

woutware commented Sep 5, 2023

Looks great Vincent!

@Viincenttt
Copy link
Owner

This is now live in 3.1.0.0
https://github.com/Viincenttt/MollieApi/releases/tag/3.1.0.0

@woutware
Copy link
Author

Just a nitpicky comment about the wording of "one will be created and disposed for you." in the home page tutorial. The caller has to explicitly call Dispose() himself (through the use of "using"), so it's not being magically being disposed without the caller calling Dispose().

A completely unrelated comment (about the documentation): it would be useful to demonstrate the use of PaymentResponse.Links.Checkout.Href in the "Payment API" paragraph. I think most users will use the Mollie payment pages and will have to redirect to this url. It's no big deal, because I found it in 10 minutes in the mollie docs, but if this would have been included in the howto, it would have been already usable right out of the box to do a complete payment.

@Viincenttt
Copy link
Owner

Thanks for the feedback @woutware , I really appreciate it! :)

I'll update the documentation to make this part a bit more clear

@woutware
Copy link
Author

The wording is much better now! And the checkout url paragraph looks good too, overall great job on the documentation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants