search

brick / money

A money and currency library for PHP
MIT License
1.61k stars 96 forks source link

florianv/swap integration #42

Closed ostrolucky closed 3 years ago

ostrolucky commented 3 years ago

I like this library, but one thing which is putting me off a bit compared to moneyphp/money is that this library doesn't have integration for most popular exchange rates provider florianv/swap. This is in contrast with moneyphp/money which comes with support bundled in.

So, hereby, this is a request for adding core support for florianv/swap as a new ExchangeRateProvider.

BenMorel commented 3 years ago

Hi, thanks for the pointer. I've never used Swap myself but it does look like a nice library.

I'm a bit concerned with having a dependency (even an optional one) on a particular third-party library, though. I like the fact that the library is only dependent on core PHP features, and provides just the interface for you to build upon.

The implementation should be just a couple lines of code, and should be trivial for you to write, or even to provide as your own open-source package?

ostrolucky commented 3 years ago

Isn't this similar to GD/bcmath in brick/math? Those are extensions that users need to install, but are encouraged to do so. You don't require them, but do provide optional support right in core. Similar would be here with florianv/swap. Same situation is in moneyphp where they don't have this worry there. As you say, integration should be pretty simple, so IMHO you don't have to be worried too much about possible interface changes.

BenMorel commented 3 years ago

In additions to being extensions, GD / bcmath implementations are quite frozen in stone, unlike userland libraries.

Again, the implementation of an adapter from brick/money to Swap is so trivial, I don't understand how this could limit adoption of brick/money? Is there a problem with adding these couple lines of code to your app?

I'm not against providing the implementation somewhere (a gist, for example, or your own repo if you wish) and link to it from the README, but I don't think it belongs to this repo.

ostrolucky commented 3 years ago

alright, let's close then

BenMorel commented 3 years ago

Feel free to create an adapter that bridges brick/money to florianv/swap, publish it to Packagist, and I'll happily link to it! :+1: