Viincenttt / MollieApi

This project allows you to easily add the Mollie payment provider to your application.
MIT License
147 stars 85 forks source link

IPaymentClient should inherit from IDisposable #319

Closed woutware closed 11 months ago

woutware commented 12 months ago

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 commented 11 months ago

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 commented 11 months ago

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

woutware commented 11 months ago

Looks great Vincent!

Viincenttt commented 11 months ago

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

woutware commented 11 months ago

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 commented 11 months ago

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

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

woutware commented 11 months ago

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