-
Notifications
You must be signed in to change notification settings - Fork 446
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
Bitcoin not supported by stringToUnits() #97
Comments
Hi @thedavidmeister maybe this could be interesting for you: |
yeah, that does look interesting. I'm just about to roll a release for something I'm working on that uses the current Money setup, so I don't want to start a refactor now. But it's something worth following up as I had to jump through some hoops along the way that I would have expected Money to handle better. |
The <?php
use Money\Currency;
use Money\Money;
use Money\MoneyParser;
class BitcoinSupportedMoneyParser implements MoneyParser
{
private $innerParser;
public function __construct(MoneyParser $innerParser)
{
$this->innerParser = $innerParser;
}
public function parse($formattedMoney, $forceCurrency = null)
{
if (strpos($formattedMoney, "\0xC9\0x83") === false) {
return $this->innerParser->parse($formattedMoney, $forceCurrency);
}
// bitcoin parsing here
return new Money($formattedMoney, new Currency('XBT'));
}
} |
@sagikazarmark This one can be closed too |
Hm, should we include this in the repo? |
Why would we want include it? By using the intl parser together with the parser interface, the user has all the possibilities to implement such a bitcoin parser of his own. If we include this one, what is then the boundary what to include and what not? |
And if we include a bitcoin parser, we should - by means of consistency - also include a formatter. Maybe we avoid getting same questions by including one. I am happy to PR both formatter and parser. Can you let me know what your decision is @sagikazarmark? |
Well, it seems bitcoin is a widely requested feature. Further more, it is plain PHP code without any dependencies. Given these two details, I don't think it's a problem to include it. |
Ok, will PR next week. |
@josecelano @thedavidmeister Could you please have a look at PR #155? Thanks. |
Recommitted to master. Who can do a review on the bitcoin parser and formatter? We need someone who deals with this on a regular basis. |
Because stringToUnits() explicitly expects only 2 trailing decimals, it cannot support Bitcoin and we see "The value could not be parsed as money"
The text was updated successfully, but these errors were encountered: